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

MarkerService and ProblemsView do not scale well and block UI thread #11976

Closed
DustinCampbell opened this issue Sep 13, 2016 · 10 comments
Closed
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

Comments

@DustinCampbell
Copy link
Member

  • VSCode Version: 1.5.2
  • OS Version: Tested Windows 10 and macOS 10.11

Repro steps:

  1. Clone https://github.com/DustinCampbell/diagnostic-perf-test
  2. Open the diagnostic-perf-test project in VS Code
  3. F5 to compile and launch the extension under the debugger
  4. In the debuggee process, select View->Problems to display the Problems pane.
  5. In the debuggee process, select View->Command Palette... and execute the Populate Diagnostics command.

At this point, the VS Code debuggee window will stop responding as it populates a single diagnostic for 2000 files. This is very similar to the diagnostics produced for a medium- to large-sized C# solution (say, one generated by Unity) where there are a couple thousand files and each has a single "remove unnecessary using directives" diagnostic.

Expected result:

  • Populating diagnostics should not scale exponentially by the number of files.
  • Calling DiagnosticCollection.set(...) should periodically free up the UI thread while processing.
@dbaeumer dbaeumer removed their assignment Sep 14, 2016
@dbaeumer dbaeumer added api bug Issue identified by VS Code Team member as probable bug labels Sep 14, 2016
@jrieken jrieken added this to the September 2016 milestone Sep 19, 2016
@jrieken jrieken added freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues and removed api labels Sep 19, 2016
@jrieken
Copy link
Member

jrieken commented Sep 19, 2016

The problem is that we call changeOne per uri+owner combination which leads to many events. Processing those events (problems view) takes a long time. We should not fire so many events...

CPU-20160919T115033.cpuprofile.zip

screen shot 2016-09-19 at 11 54 19

@jrieken
Copy link
Member

jrieken commented Sep 19, 2016

We have found a couple of issues

  • event storm by emitting in the loop
  • the problems view sorting and filtering all elements

@jrieken
Copy link
Member

jrieken commented Sep 19, 2016

@sandy081 I have pushed changes to send all events at once and to make reading of markers a little faster. that improves the situation a tiny bit. as discussed the culprit is the problems view. pls continue from here

@sandy081
Copy link
Member

Thanks @jrieken for the changes. I will do the improvement is problems view as discussed.

@jrieken jrieken removed their assignment Sep 20, 2016
@sandy081
Copy link
Member

sandy081 commented Sep 20, 2016

Root cause: Problems model is getting updated resource by resource and every time it is getting refreshed (filtered and sorted). So this is doing filtering and sorting as many times as the number of resources (in this case 2000). Fixed it by doing bulk update / refresh.

Also did some optimisations in the model for better performance. After all this, amount of time came down from 20s to 1s

Attaching the pre-fix and post-fix profiles here

screen shot 2016-09-20 at 18 39 18

screen shot 2016-09-20 at 19 26 15

@DustinCampbell
Copy link
Member Author

nice!

@jrieken jrieken reopened this Sep 21, 2016
@jrieken jrieken assigned jrieken and unassigned sandy081 Sep 21, 2016
@jrieken
Copy link
Member

jrieken commented Sep 21, 2016

reopening for me to further improve MarkerService#read. That 1 second must go ;-)

@jrieken
Copy link
Member

jrieken commented Sep 22, 2016

🐎 MarkerService#read from 1000ms down to 2.5ms. Slightly embarrassing it wasn't like this to begin with...

screen shot 2016-09-22 at 09 16 01

@jrieken
Copy link
Member

jrieken commented Sep 22, 2016

@sandy081 The last big chunk is Tree#expand which you might wanna consult with @joaomoreno if it can be improved.

screen shot 2016-09-22 at 09 19 51

@jrieken jrieken removed their assignment Sep 22, 2016
@sandy081
Copy link
Member

sandy081 commented Sep 28, 2016

I added another fix/improvement while applying filters.

Using Tree.expand on each item or Tree.expandAll is not making any difference.
Will discuss with @joaomoreno and see if I can improve this.

Otherwise Closing this since we improved performance drastically.

@jrieken jrieken changed the title DiagnosticCollection does not scale well and blocks UI thread MarkerService and ProblemsView do not scale well and block UI thread Sep 28, 2016
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
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
Projects
None yet
Development

No branches or pull requests

4 participants