-
Notifications
You must be signed in to change notification settings - Fork 336
feat: improve performance of the breakpoint predictor #111
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
Conversation
This commit is a performance pass on the breakpoint predictor. There are two main changes. First, it uses ripgrep instead of our Node.js grepping strategy, when possible. This benefits us in two ways: it avoids having the breakpoint prediction logic competing for resources with the interim runtime breakpoint determination logic; and of course it takes advantage of an incredibly well-tuned, multicore tool. Because we don't know what platform the tool will run on, we use a new library[1] to download the appropriate prebuilt binary into a directory sent in the launch config. (I'll publish that as a module if we decide this is the direction we want to go). Eric mentioned for VS that they could consider shipping the win32 binaries inside the vsix. Regardless, though, if the ripgrep storage dir isn't set, we'll fall back to the current Node adapter. Second, we cache resolved sourcemap information based on the file modified time, and run the resolution in parallel. This nets us another solid performance improvement. Overall, I'm seeing the following numbers for predictor runtime. ``` before changes: ~6000ms node.js discovery: ~3000ms (+ caching) ripgrep discovery: ~800ms (+ caching, ripgrep) ``` 1. https://github.com/microsoft/vscode-ripgrep-runtime
7ff53af to
22e1cb5
Compare
|
I really don't think we should be downloading ripgrep in the vscode extension, especially when it's a builtin extension and vscode already includes ripgrep. We should use the proposed api to run a text search. It might be slightly slower due to having to having more code and process boundaries between points a and b, but it just feels very wrong to do this. |
d9a0156 to
d69a8f9
Compare
by moving the generated file tree inside the workspace
|
The problem with the findTextInFiles API is that it can't take absolute paths and can't search outside of the current workspace. I fixed the tests by moving the test workspace inside the current workspace, but realized I haven't fixed the actual product and I'm going to just tackle the real problem in vscode. Will likely revert my last commit if I do. |
need to separate the folder/pattern in the include pattern passed in. Also, force the test extension dev host to open a folder
This commit is a performance pass on the breakpoint predictor. There
are two main changes.
First, it uses ripgrep instead of our Node.js grepping strategy, when
possible. This benefits us in two ways: it avoids having the breakpoint
prediction logic competing for resources with the interim runtime
breakpoint determination logic; and of course it takes advantage of an
incredibly well-tuned, multicore tool.
Because we don't know what platform the tool will run on, we use a
new library[1] to download the appropriate prebuilt binary into a
directory sent in the launch config. (I'll publish that as a module if
we decide this is the direction we want to go). Eric mentioned for VS
that they could consider shipping the win32 binaries inside the vsix.
Regardless, though, if the ripgrep storage dir isn't set, we'll fall
back to the current Node adapter.
Second, we cache resolved sourcemap information based on the file
modified time, and run the resolution in parallel. This nets us another
solid performance improvement.
Overall, I'm seeing the following numbers for predictor runtime.