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

Editor hang/poor performance with a large number of tests in a file #170493

Closed
DanTup opened this issue Jan 3, 2023 · 2 comments · Fixed by #173082
Closed

Editor hang/poor performance with a large number of tests in a file #170493

DanTup opened this issue Jan 3, 2023 · 2 comments · Fixed by #173082
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues insiders-released Patch has been released in VS Code Insiders testing Built-in testing support verified Verification succeeded
Milestone

Comments

@DanTup
Copy link
Contributor

DanTup commented Jan 3, 2023

I'm investigating some performance issues in Dart when you have a large number of tests in a single file. Many of these issues are in the Dart extension, but while working on it I've been seeing a few bad hands in the editor that I think may be a VS Code issue. It seems to happen during syncDecorations which I suspect are related to the test icons in the gutter.

The test file I'm using has approximately 2,500 tests in the file.

Screenshot 2023-01-03 at 15 12 51

Screenshot 2023-01-03 at 15 17 03

Here's an export of the profile from the dev tools:

slow_decorations.json.zip

Note: I can't give precise repro steps using the Dart extension right now because I'm still making performance improvements (if you try to reproduce this with the published version of the extension you'll see significant performance issues in the extension that I'm working on), but if this profile isn't helpful on its own and you need more, I may be in a position to publish a pre-release version with some of my fixes soon.

@connor4312 FYI - I don't know if this is general decorations code or test-specific, but I think the anonymous function showing up a lot in the profile relates to this code here:

const map = model.changeDecorations(accessor => {

@connor4312 connor4312 added bug Issue identified by VS Code Team member as probable bug freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues testing Built-in testing support labels Jan 3, 2023
@connor4312
Copy link
Member

connor4312 commented Feb 1, 2023

Repro steps:

  1. Use the test provider sample
node -p "new Array(2500).fill(0).map((a, i) => \`1 + \${i} = \${i + 1}\`).join('\n')" > sample/big.md
  1. F5, open big.md, and click on a decoration

@connor4312 connor4312 added this to the February 2023 milestone Feb 1, 2023
connor4312 added a commit that referenced this issue Feb 1, 2023
Fixes #170493. Goes from a 1600ms hang to an 8ms computation.
connor4312 added a commit that referenced this issue Feb 2, 2023
Fixes #170493. Goes from a 1600ms hang to an 8ms computation.
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Feb 2, 2023
@DanTup
Copy link
Contributor Author

DanTup commented Feb 8, 2023

@connor4312 seems much better now (in Insiders), thank you!

@aeschli aeschli added the verified Verification succeeded label Feb 23, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues insiders-released Patch has been released in VS Code Insiders testing Built-in testing support verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants