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 breakpoint predictor caching for large projects #1149

Closed
roblourens opened this issue Dec 4, 2021 · 4 comments · Fixed by #1498
Closed

Improve breakpoint predictor caching for large projects #1149

roblourens opened this issue Dec 4, 2021 · 4 comments · Fixed by #1498
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone

Comments

@roblourens
Copy link
Member

I sometimes see breakpoints being very slow to be set when debugging vscode. Here's a log where it took several seconds for the breakpoint to become solid red. This is using the normal "VS Code" launch config and code-oss was not very busy at the moment I set the breakpoint in searchView.ts.

vscode-debugadapter-bd6f7e46.json.gz

@roblourens roblourens added the bug Issue identified by VS Code Team member as probable bug label Dec 4, 2021
@seanfoltz

This comment has been minimized.

@connor4312
Copy link
Member

It looks like that is just time in the breakpoint predictor. Just doing some quick local tests, essentially all the time is spent doing the file search.

I think there is possibly some smarter caching that can be done if we avoid ripgrep, for example by knowing file mtimes, remembering the last sourcemap we saw if the mtime matches (which we do today, but only after discovering the sourcemap string), and not bothering reading the file if it doesn't match.

@connor4312 connor4312 changed the title Slow breakpoints Improve breakpoint predictor caching for large projects Dec 17, 2021
@joaomoreno joaomoreno added this to the June 2022 milestone Jun 8, 2022
@connor4312 connor4312 modified the milestones: June 2022, July 2022 Jun 29, 2022
@joaomoreno
Copy link
Member

Is this maybe a dupe of microsoft/vscode#153470?

@connor4312
Copy link
Member

This was mostly covered by the PR I sent to fix that issue, but there are still a few more things I want to take care of

@connor4312 connor4312 modified the milestones: July 2022, August 2022 Jul 29, 2022
@lszomoru lszomoru modified the milestones: September 2022, October 2022 Oct 5, 2022
@connor4312 connor4312 modified the milestones: October 2022, On Deck Oct 25, 2022
connor4312 added a commit that referenced this issue Dec 18, 2022
For #1149

This does some overall system optimization for how the breakpoint
predictor works. It achieves performance comparable to the VS Code API
ripgrep globbing, with plain JS. And by using plain JS, we can do more
optimizations in a future PR.

Previously, we had an `ISearchStrategy` type that globbed for all
sourcemap URIs in a target folder, and emitted those. Then the breakpoint
predictor had an mtime-based cache to decide whether it would parse the
sourcemap or not.

This moves the caching responsibility into `ISearchStrategy`, which then
takes a two-phase approach to processing files, allowing it to cache the
intermediate result the breakpoint predictor previously cached itself.
This means it doesn't need to read files at all if the mtime doesn't
change.

There remains some lower-level optimization: I'm going to improve how the
cache results get stored (currently the cache file for VS code is 10's
of MB of JSON.)

Later, I'm going to improve how the source map URL is actually read from
the file. Today we read the entire file into a string, but we can improve
this significantly, since, by spec, the sourceMapURL is always at the
end. That was the original reason I broke from the ripgrep-based
approach, since ripgrep greps the whole file.

![](https://media4.giphy.com/media/cRH5deQTgTMR2/giphy.gif?cid=ecf05e47r7gdpd1vzjtyo72z0ymmob3ltfjz7ts4cs64n18i&rid=giphy.gif&ct=g)
connor4312 added a commit that referenced this issue Dec 29, 2022
For #1149

This does some overall system optimization for how the breakpoint
predictor works. It achieves performance comparable to the VS Code API
ripgrep globbing, with plain JS. And by using plain JS, we can do more
optimizations in a future PR.

Previously, we had an `ISearchStrategy` type that globbed for all
sourcemap URIs in a target folder, and emitted those. Then the breakpoint
predictor had an mtime-based cache to decide whether it would parse the
sourcemap or not.

This moves the caching responsibility into `ISearchStrategy`, which then
takes a two-phase approach to processing files, allowing it to cache the
intermediate result the breakpoint predictor previously cached itself.
This means it doesn't need to read files at all if the mtime doesn't
change.

There remains some lower-level optimization: I'm going to improve how the
cache results get stored (currently the cache file for VS code is 10's
of MB of JSON.)

Later, I'm going to improve how the source map URL is actually read from
the file. Today we read the entire file into a string, but we can improve
this significantly, since, by spec, the sourceMapURL is always at the
end. That was the original reason I broke from the ripgrep-based
approach, since ripgrep greps the whole file.

![](https://media4.giphy.com/media/cRH5deQTgTMR2/giphy.gif?cid=ecf05e47r7gdpd1vzjtyo72z0ymmob3ltfjz7ts4cs64n18i&rid=giphy.gif&ct=g)
connor4312 added a commit that referenced this issue Dec 30, 2022
Fixes #1149
Fixes microsoft/vscode#167911

This does some overall system optimization for how the breakpoint
predictor works. It achieves performance comparable to the VS Code API
ripgrep globbing, with plain JS. And by using plain JS, we can do more
optimizations in a future PR.

Previously, we had an `ISearchStrategy` type that globbed for all
sourcemap URIs in a target folder, and emitted those. Then the breakpoint
predictor had an mtime-based cache to decide whether it would parse the
sourcemap or not.

This moves the caching responsibility into `ISearchStrategy`, which then
takes a two-phase approach to processing files, allowing it to cache the
intermediate result the breakpoint predictor previously cached itself.

This means that:

- Files never need to be read if their mtime doesn't change
- When starting a new child session, we optimize further and don't stat
  compiled files that didn't previously reference the sourcefile. (We
  still stat all directories to see if there are new files to pick up)

Some performance numbers:

- vscode#167911 time on my computer going from 33s -> 16s
- typescript: time to run mocha tests

The new search strategy can be disabled by setting
`enableTurboSourcemaps: false` in your launch.json.

![](https://media4.giphy.com/media/cRH5deQTgTMR2/giphy.gif?cid=ecf05e47r7gdpd1vzjtyo72z0ymmob3ltfjz7ts4cs64n18i&rid=giphy.gif&ct=g)
connor4312 added a commit that referenced this issue Dec 30, 2022
Fixes #1149
Fixes microsoft/vscode#167911

This does some overall system optimization for how the breakpoint
predictor works. It achieves performance comparable to the VS Code API
ripgrep globbing, with plain JS. And by using plain JS, we can do more
optimizations in a future PR.

Previously, we had an `ISearchStrategy` type that globbed for all
sourcemap URIs in a target folder, and emitted those. Then the breakpoint
predictor had an mtime-based cache to decide whether it would parse the
sourcemap or not.

This moves the caching responsibility into `ISearchStrategy`, which then
takes a two-phase approach to processing files, allowing it to cache the
intermediate result the breakpoint predictor previously cached itself.

This means that:

- Files never need to be read if their mtime doesn't change
- When starting a new child session, we optimize further and don't stat
  compiled files that didn't previously reference the sourcefile. (We
  still stat all directories to see if there are new files to pick up)

Some performance numbers:

- vscode#167911 time on my computer going from 33s -> 16s
- typescript: time to run mocha tests 52s -> 22s (without outFiles configured)

The new search strategy can be disabled by setting
`enableTurboSourcemaps: false` in your launch.json.

![](https://media4.giphy.com/media/cRH5deQTgTMR2/giphy.gif?cid=ecf05e47r7gdpd1vzjtyo72z0ymmob3ltfjz7ts4cs64n18i&rid=giphy.gif&ct=g)
connor4312 added a commit that referenced this issue Dec 30, 2022
Fixes #1149
Fixes microsoft/vscode#167911

This does some overall system optimization for how the breakpoint
predictor works. It achieves performance comparable to the VS Code API
ripgrep globbing, with plain JS. And by using plain JS, we can do more
optimizations in a future PR.

Previously, we had an `ISearchStrategy` type that globbed for all
sourcemap URIs in a target folder, and emitted those. Then the breakpoint
predictor had an mtime-based cache to decide whether it would parse the
sourcemap or not.

This moves the caching responsibility into `ISearchStrategy`, which then
takes a two-phase approach to processing files, allowing it to cache the
intermediate result the breakpoint predictor previously cached itself.

This means that:

- Files never need to be read if their mtime doesn't change
- When starting a new child session, we optimize further and don't stat
  compiled files that didn't previously reference the sourcefile. (We
  still stat all directories to see if there are new files to pick up)

Some performance numbers:

- vscode#167911 time on my computer going from 33s -> 16s
- typescript: time to run mocha tests 52s -> 22s (without outFiles configured)

The new search strategy can be disabled by setting
`enableTurboSourcemaps: false` in your launch.json.

![](https://media4.giphy.com/media/cRH5deQTgTMR2/giphy.gif?cid=ecf05e47r7gdpd1vzjtyo72z0ymmob3ltfjz7ts4cs64n18i&rid=giphy.gif&ct=g)
@connor4312 connor4312 removed this from the On Deck milestone Dec 30, 2022
@connor4312 connor4312 added this to the January 2023 milestone Dec 30, 2022
@roblourens roblourens added the verified Verification succeeded label Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants