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

Improve semantic token latency, highlight function and modified variables #135

Merged
merged 3 commits into from
Sep 30, 2024

Conversation

6cdh
Copy link
Contributor

@6cdh 6cdh commented Sep 22, 2024

This PR added highlight for function and modified variables, and improved expand performance by 30% to 10x.

The details as follows:

  • For typed racket, replace its language line with no-check language line before expand. It avoid type check and reduce at least one second for each expand. If error happens because of the replace, it would fallback to normal language line.
  • Cache namespace. It optimizes for a common scene when people only concentrates on a single file, and don't open other files during the period. In this case, it caches and reuses the namespace of last expand.

This snippet is a demo. For example:

$ racket fast-expand.rkt path/to/racket-langserver/main.rkt
expand
cpu time: 2196 real time: 2196 gc time: 881
expand2
cpu time: 191 real time: 191 gc time: 71

Here the second expand is faster than the first expand by reuses the namespace.

And a typed racket file without the nocheck replacement:

$ racket fast-expand.rkt path/to/typed-racket/typed-racket-test/gui/succeed/racket-esquire.rkt
expand
cpu time: 2244 real time: 2455 gc time: 792
expand2
cpu time: 1305 real time: 1305 gc time: 392

with nocheck replacement:

$ racket fast-expand.rkt path/to/typed-racket/typed-racket-test/gui/succeed/racket-esquire.rkt
expand
cpu time: 892 real time: 1049 gc time: 324
expand2
cpu time: 226 real time: 226 gc time: 68

expand.rkt Outdated Show resolved Hide resolved
expand.rkt Show resolved Hide resolved
@dannypsnl
Copy link
Contributor

dannypsnl commented Sep 23, 2024

I'm wondering how would an invalid namespace behaved? There are two possibility in my mind

  1. It throw an exception
  2. It still working but produces wrong result

The first one is actually help us to detect it then provide a new one, the second one is harder to handle (wait me modify the snippet & test).

Update

I random pick a module, insert a new definition and provide it, cannot get exception in this way.

@6cdh
Copy link
Contributor Author

6cdh commented Sep 23, 2024

I guess it might throw a exception or silently produce wrong results in different cases.

Because a namespace contains mappings from module names to module bindings. If a module x requires module y, we cache the namespace, then

  • change module y's implementation and keep its interface (provided bindings), then it probably produces wrong result.
  • change y's interface to provide a subset of old interface, then modify module x. It would produce correct result.
  • provide a superset of old interface, then modify module x. It probably throws an undefined identifier error.
  • more complex scenario ...

@dannypsnl
Copy link
Contributor

Yep, maybe we can create an issue for further discussion, for now the PR is great.

@6cdh
Copy link
Contributor Author

6cdh commented Sep 29, 2024

The code to identify function token was complex and has a bug that treat the expression (if ... proc1 proc2) in ((if ... proc1 proc2) arg) as function. I just fixed it.

@jeapostrophe jeapostrophe merged commit 7481d2f into jeapostrophe:master Sep 30, 2024
5 checks passed
@jeapostrophe
Copy link
Owner

Thanks!

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