optimize text document cache and data algorithms

Author: d-biehl

robotcode/language_server/common/parts/workspace.py

@@ -408,7 +408,7 @@ async def add_file_watchers(
 
         def remove() -> None:
             if self._loop.is_running():
-                asyncio.run_coroutine_threadsafe(self.remove_file_watcher_entry(entry), self._loop).result()
+                asyncio.run_coroutine_threadsafe(self.remove_file_watcher_entry(entry), self._loop)
 
         weakref.finalize(entry, remove)
 

robotcode/language_server/common/text_document.py

@@ -57,6 +57,7 @@ def __init__(
         super().__init__()
 
         self._lock = asyncio.Lock()
+
         self._references: weakref.WeakSet[Any] = weakref.WeakSet()
 
         self.document_uri = (
@@ -84,8 +85,6 @@ def __init__(
 
         self._data: weakref.WeakKeyDictionary[Any, Any] = weakref.WeakKeyDictionary()
 
-        self._loop = asyncio.get_event_loop()
-
     @property
     def references(self) -> weakref.WeakSet[Any]:  # pragma: no cover
         return self._references
@@ -180,22 +179,24 @@ async def invalidate_data(self) -> None:
         async with self._lock:
             self._invalidate_data()
 
-    async def __remove_cache_entry_safe(self, ref: Any) -> None:
-        async with self._lock:
-            self._cache.pop(ref)
-
     def __remove_cache_entry(self, ref: Any) -> None:
-        if self._loop is not None and self._loop.is_running():
-            asyncio.run_coroutine_threadsafe(self.__remove_cache_entry_safe(ref), self._loop)
+        async def __remove_cache_entry_safe(_ref: Any) -> None:
+            async with self._lock:
+                self._cache.pop(_ref)
+
+        if self._lock.locked():
+            asyncio.create_task(__remove_cache_entry_safe(ref))
         else:
-            self._cache.pop(ref)  # pragma: no cover
+            self._cache.pop(ref)
 
-    def __get_cache_reference(self, entry: Callable[..., Any]) -> weakref.ref[Any]:
+    def __get_cache_reference(self, entry: Callable[..., Any], /, *, add_remove: bool = True) -> weakref.ref[Any]:
 
         if inspect.ismethod(entry):
-            reference: weakref.ref[Any] = weakref.WeakMethod(cast(MethodType, entry), self.__remove_cache_entry)
+            reference: weakref.ref[Any] = weakref.WeakMethod(
+                cast(MethodType, entry), self.__remove_cache_entry if add_remove else None
+            )
         else:
-            reference = weakref.ref(entry, self.__remove_cache_entry)
+            reference = weakref.ref(entry, self.__remove_cache_entry if add_remove else None)
 
         return reference
 
@@ -226,10 +227,9 @@ async def remove_cache_entry(
         self, entry: Union[Callable[[TextDocument], Awaitable[_T]], Callable[..., Awaitable[_T]]]
     ) -> None:
         async with self._lock:
-            if inspect.ismethod(entry):
-                self._cache.pop(weakref.WeakMethod(cast(MethodType, entry)), None)
-            else:
-                self._cache.pop(weakref.ref(entry), None)
+            self.__remove_cache_entry(self.__get_cache_reference(entry, add_remove=False))
+
+        await asyncio.sleep(0)
 
     def set_data(self, key: Any, data: Any) -> None:
         self._data[key] = data

tests/robotcode/language_server/common/test_text_document.py

@@ -1,7 +1,3 @@
-import asyncio
-from asyncio.events import AbstractEventLoop
-from typing import Generator
-
 import pytest
 
 from robotcode.language_server.common.lsp_types import Position, Range
@@ -11,13 +7,6 @@
 )
 
 
-@pytest.fixture
-def event_loop() -> Generator[AbstractEventLoop, None, None]:
-    loop = asyncio.new_event_loop()
-    yield loop
-    loop.close()
-
-
 @pytest.mark.asyncio
 async def test_apply_full_change_should_work() -> None:
     text = """first"""
@@ -141,17 +130,17 @@ async def test_lines_should_give_the_lines_of_the_document() -> None:
     assert document.lines == text.splitlines(True)
 
 
-class WeakReferencable:
-    pass
-
-
 @pytest.mark.asyncio
 async def test_document_get_set_clear_data_should_work() -> None:
     text = """\
 first
 second
 third
 """
+
+    class WeakReferencable:
+        pass
+
     key = WeakReferencable()
     data = "some data"
 
@@ -162,6 +151,11 @@ async def test_document_get_set_clear_data_should_work() -> None:
     await document.clear()
     assert document.get_data(key, None) is None
 
+    document.set_data(key, data)
+    assert document.get_data(key) == data
+    await document.invalidate_data()
+    assert document.get_data(key, None) is None
+
 
 @pytest.mark.asyncio
 async def test_document_get_set_cache_with_function_should_work() -> None:
@@ -170,16 +164,29 @@ async def test_document_get_set_cache_with_function_should_work() -> None:
 second
 third
 """
+    prefix = "1"
 
     async def get_data(document: TextDocument, data: str) -> str:
-        return "blah" + data
+        return prefix + data
 
     document = TextDocument(document_uri="file://test.robot", language_id="robotframework", version=1, text=text)
 
-    assert await document.get_cache(get_data, "data") == "blahdata"
+    assert await document.get_cache(get_data, "data") == "1data"
+
+    prefix = "2"
+    assert await document.get_cache(get_data, "data1") == "1data"
 
     await document.remove_cache_entry(get_data)
 
+    assert await document.get_cache(get_data, "data2") == "2data2"
+
+    prefix = "3"
+    assert await document.get_cache(get_data, "data3") == "2data2"
+
+    await document.invalidate_cache()
+
+    assert await document.get_cache(get_data, "data3") == "3data3"
+
 
 @pytest.mark.asyncio
 async def test_document_get_set_cache_with_method_should_work() -> None:
@@ -190,13 +197,48 @@ async def test_document_get_set_cache_with_method_should_work() -> None:
 """
     document = TextDocument(document_uri="file://test.robot", language_id="robotframework", version=1, text=text)
 
+    prefix = "1"
+
     class Dummy:
         async def get_data(self, document: TextDocument, data: str) -> str:
-            return "blah" + data
+            return prefix + data
 
     dummy = Dummy()
 
-    assert await document.get_cache(dummy.get_data, "data") == "blahdata"
+    assert await document.get_cache(dummy.get_data, "data") == "1data"
+
+    prefix = "2"
+    assert await document.get_cache(dummy.get_data, "data1") == "1data"
+
+    await document.remove_cache_entry(dummy.get_data)
+
+    assert await document.get_cache(dummy.get_data, "data2") == "2data2"
+
+    prefix = "3"
+    assert await document.get_cache(dummy.get_data, "data3") == "2data2"
+
+    await document.invalidate_cache()
+
+    assert await document.get_cache(dummy.get_data, "data3") == "3data3"
 
-    # await document.remove_cache_entry(dummy.get_data)
     del dummy
+
+    assert len(document._cache) == 0
+
+
+@pytest.mark.asyncio
+async def test_document_get_set_cache_with_lock_work() -> None:
+    text = """\
+first
+second
+third
+"""
+    prefix = "1"
+
+    async def get_data(document: TextDocument, data: str) -> str:
+        await document.remove_cache_entry(get_data)
+        return prefix + data
+
+    document = TextDocument(document_uri="file://test.robot", language_id="robotframework", version=1, text=text)
+
+    assert await document.get_cache(get_data, "data") == "1data"

tests/robotcode/language_server/robotframework/conftest.py

@@ -1,4 +1,5 @@
 import asyncio
+import logging
 from pathlib import Path
 from typing import Any, AsyncGenerator, Generator, cast
 
@@ -26,14 +27,24 @@
 from robotcode.utils.dataclasses import as_dict
 
 
-@pytest.fixture(scope="module")
+@pytest.fixture(scope="function")
 def event_loop() -> Generator[asyncio.AbstractEventLoop, None, None]:
+    logging.info("create event_loop")
     loop = asyncio.new_event_loop()
-    yield loop
-    loop.close()
+    try:
+        yield loop
+    finally:
+        for t in asyncio.all_tasks(loop):
+            logging.error(t)
+
+        logging.info("event_loop close")
+
+        # loop.close()
+
+        logging.info("event_loop closed")
 
 
-@pytest.fixture(scope="module")
+@pytest.fixture(scope="function")
 async def protocol(event_loop: asyncio.AbstractEventLoop) -> AsyncGenerator[RobotLanguageServerProtocol, None]:
     root_path = Path().resolve()
     server = RobotLanguageServer()
@@ -70,7 +81,7 @@ async def protocol(event_loop: asyncio.AbstractEventLoop) -> AsyncGenerator[Robo
         server.close()
 
 
-@pytest.fixture(scope="module")
+@pytest.fixture(scope="function")
 async def test_document(event_loop: asyncio.AbstractEventLoop, request: Any) -> AsyncGenerator[TextDocument, None]:
     data_path = Path(request.param)
     data = data_path.read_text()
@@ -81,4 +92,5 @@ async def test_document(event_loop: asyncio.AbstractEventLoop, request: Any) ->
     try:
         yield document
     finally:
-        del document
+        # del document
+        pass

tests/robotcode/language_server/robotframework/parts/test_goto.py

@@ -22,8 +22,8 @@
     indirect=["test_document"],
     ids=generate_test_id,
 )
-@pytest.mark.asyncio
 @pytest.mark.usefixtures("protocol")
+@pytest.mark.asyncio
 async def test_goto(
     protocol: RobotLanguageServerProtocol,
     test_document: TextDocument,

tests/robotcode/language_server/robotframework/parts/test_hover.py

@@ -22,8 +22,8 @@
     indirect=["test_document"],
     ids=generate_test_id,
 )
-@pytest.mark.asyncio
 @pytest.mark.usefixtures("protocol")
+@pytest.mark.asyncio
 async def test_hover(
     protocol: RobotLanguageServerProtocol,
     test_document: TextDocument,