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

Tree slow! #18180

Closed
jrieken opened this issue Jan 5, 2017 · 7 comments
Closed

Tree slow! #18180

jrieken opened this issue Jan 5, 2017 · 7 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debt Code quality issues perf tree-widget Tree widget issues verified Verification succeeded
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Jan 5, 2017

Look at the screen shot form the attached profile. I might be reading this wrong, but it seems that the tree looses 10ms between expandAll and onModelEvents but just emitting events to/from itself.

screen shot 2017-01-05 at 17 53 52

CPU-20170105T174926.cpuprofile.zip

@joaomoreno
Copy link
Member

Repro steps?

@joaomoreno joaomoreno added this to the Backlog milestone Jan 6, 2017
@jrieken
Copy link
Member Author

jrieken commented Jan 6, 2017

Open explorer with 50 items, start profiler, reload workbench

@joaomoreno
Copy link
Member

Are they all direct children of the root? Are some expanded?

@jrieken
Copy link
Member Author

jrieken commented Jan 6, 2017

@joaomoreno Unsure what your question meant, but notice that the profile is attached to this issue

@joaomoreno joaomoreno added the debt Code quality issues label Jan 6, 2017
@joaomoreno joaomoreno changed the title Tree slow? Tree slow! Jan 6, 2017
@joaomoreno
Copy link
Member

For the tree, we gotta get rid of

  1. Promises
  2. Events

@jrieken
Copy link
Member Author

jrieken commented May 17, 2018

I don't think that making each Item be 12 emitters helped in anyway: https://github.com/Microsoft/vscode/blob/master/src/vs/base/parts/tree/browser/treeModel.ts#L267

Given the new outline:

  • open lib.d.ts
  • type 'A'
  • wait
  • erase 'A'
  • wait, wait, wait, 🐌, 😴 , 🐢 , 🚜...
  • it takes 10 seconds and longer

screen shot 2018-05-17 at 15 57 33

@joaomoreno
Copy link
Member

joaomoreno commented Oct 15, 2018

Nothing will be done on the old tree.

A few new tree widgets have started to appear in our code base: IndexTree, ObjectTree and DataTree, all of them considerably improving performance across all features, the former 2 implementations completely dropping promises and events. together, they provide the same feature set as the old tree. New issues will be created for adopting the new tree widgets in order to benefit from this.

@joaomoreno joaomoreno modified the milestones: Backlog, October 2018 Oct 15, 2018
@octref octref added the verified Verification succeeded label Nov 1, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 29, 2018
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 debt Code quality issues perf tree-widget Tree widget issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants