-
Notifications
You must be signed in to change notification settings - Fork 265
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
fix: improve repl/variable/completion performance for TS #1473
Conversation
Fixes #1433 There were a couple problems: - TS uses a `customDescriptionGenerator`. Previously this was being invoked on each individual property. I've merged this with the logic we added for custom `toString()` representations, so that we run it only once for an object and return a map of its properties. - TS has thousands of variables in each scope. We call `Runtime.getProperties` to get scope variables to use in completions. It seems like getProperties, which does not have any 'limit' parameter, is blocking. We still need this to do sensible completions, but now each stacktrace caches its completions instead of getting new ones on every request. With these changes, there may still be a little REPL stall on the first evaluation (until the first completions call finishes, which takes 3-5s on my macbook), but after that it should be appropriately snappy.
d6e916e
to
647a933
Compare
Updated test fixtures to make tests happy. The issue was previously we showed the custom description in the |
|
||
constructor( | ||
@inject(IRenameProvider) private readonly renameProvider: IRenameProvider, | ||
@inject(ICdpApi) private readonly cdp: Cdp.Api, | ||
@inject(IDapApi) private readonly dap: Dap.Api, | ||
@inject(AnyLaunchConfiguration) private readonly launchConfig: AnyLaunchConfiguration, | ||
private readonly locationProvider: IVariableStoreLocationProvider, | ||
) {} | ||
) { | ||
this.contextSettings = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously we were code-genning every time we made a preview for a variable. Fortunately Acorn is pretty speedy, but we can precompute that instead easily :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests failing?
flake that I need to fix |
Fixes #1433
There were a couple problems:
customDescriptionGenerator
. Previously this was being invoked on each individual property. I've merged this with the logic we added for customtoString()
representations, so that we run it only once for an object and return a map of its properties.Runtime.getProperties
to get scope variables to use in completions. It seems like getProperties, which does not have any 'limit' parameter, is blocking. We still need this to do sensible completions, but now each stacktrace caches its completions instead of getting new ones on every request.With these changes, there may still be a little REPL stall on the first evaluation (until the first completions call finishes, which takes 3-5s on my macbook), but after that it should be appropriately snappy.