Skip to content

Conversation

@connor4312
Copy link
Member

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

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
@roblourens
Copy link
Member

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.

@connor4312 connor4312 changed the base branch from fix/vscode-slowness to master November 23, 2019 05:56
by moving the generated file tree inside the workspace
@roblourens
Copy link
Member

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
@connor4312 connor4312 merged commit c297325 into master Dec 2, 2019
@connor4312 connor4312 deleted the feat/more-faster branch December 2, 2019 18:42
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