Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Services #137

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Refactor Services #137

wants to merge 4 commits into from

Conversation

6cdh
Copy link
Contributor

@6cdh 6cdh commented Oct 25, 2024

This is an unexpected big PR.

At the beginning, I found the semantic token highlight service can slow down the whole lsp server, and eats a lot of memory over time. I think that because there are too much semantic token requests to be processed. So I want to move it into build-trace% class, make it runs along with check syntax task, and reduce the number of requests.

The problem is that the semantic token request needs to be processed concurrently, and I need to add locks to a lot of code. Most changes in this PR come from the locked code.

To make the concurrent code intuitive and simple, I also considered the design again.

In this design, All requests are classified to two types of nodes:

  • text node - denotes the text state of the document. A new document change notification will produce a new text node, and make the old text node invalid.
  • query node - denotes the various read only query requests. The query nodes need to access the content of the corresponding text node to do their work.

As the graph shows:

Untitled Diagram

Any new request is either a new text node or a new query node that depends on the current text node.

The text node updates synchronously then runs a check syntax task asynchronously. Most queries also runs synchronously as before. But for async queries (only semantic token service currently), they are scheduled to run before the next text change event or after its check syntax task completed. So there are two events to trigger the process of the async queries.

In other words, a text change will clear all old queries, a successful check syntax task can also clear all old queries.

I also refactored the build-trace% class using visitor pattern. And added comment to methods.rkt to introduce the design.

The result is that the overall latency is back to previous level and the semantic tokens is faster to response.

The cached expand I added in my previous PR is useless now, but it might be useful to speedup the check syntax task in the future, so I didn't remove it.

lock.rkt Outdated Show resolved Hide resolved
tests/textDocument/formatting.rkt Show resolved Hide resolved
@jeapostrophe
Copy link
Owner

This is a pretty big change... are there any more comments from contributors before I merge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants