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

Inefficient way to create the first version of the dependencies tree after loading a project #3321

Open
lifengl opened this issue Feb 27, 2018 · 6 comments
Assignees
Labels
Feature-Dependency-Node "Dependencies" node in Solution Explorer that display project, binary & package references Performance-Scenario-Solution-Open This issue affects solution open performance. Priority:2 Work that is important, but not critical for the release Tenet-Performance This issue affects the "Performance" tenet. Triage-Approved Reviewed and prioritized
Milestone

Comments

@lifengl
Copy link
Contributor

lifengl commented Feb 27, 2018

GroupedByTargetTreeViewProvider.BuildSubTreesAsync will create nodes by nodes and insert them to the tree. It turns out to be a very inefficient way to create the immutable tree. When the code add one item, the entire spine will need be replaced (also it remembers the each change in the history record). The main project tree in CPS has been written to create it from bottom-up. (Creating leaf nodes first then folders with existing children nodes). This will reduce lots of CPU/memory overhead. The later sequence still has to update node by node because of the limitation of the current data structure, but usually, it will do much less change after first iteration.

@Pilchie Pilchie self-assigned this Feb 28, 2018
@Pilchie Pilchie added this to the 15.7 milestone Feb 28, 2018
@Pilchie Pilchie added Tenet-Performance This issue affects the "Performance" tenet. Area-New-Project-System Feature-Dependency-Node "Dependencies" node in Solution Explorer that display project, binary & package references labels Feb 28, 2018
@Pilchie
Copy link
Member

Pilchie commented Feb 28, 2018

I'll try to take a look at this.

@abpiskunov
Copy link
Contributor

abpiskunov commented Feb 28, 2018 via email

@lifengl
Copy link
Contributor Author

lifengl commented Feb 28, 2018

@abpiskunov came and told me that the tree actually was updated heavily during the loading time, which means that optimize the first load won't improve the overall perf much. In that case, it won't be worth to do the change. I agree that sounds a good explanation on why we saw a very big change history for our tree. Based on that, I agree fixing the first load doesn't make big sense, and we can punt this.

@Pilchie Pilchie assigned etbyrd and unassigned Pilchie Apr 4, 2018
@Pilchie Pilchie modified the milestones: 15.7, 15.8 Apr 4, 2018
@Pilchie Pilchie modified the milestones: 15.8, 16.0 Jul 3, 2018
@davkean davkean assigned drewnoakes and unassigned etbyrd Nov 29, 2018
@drewnoakes
Copy link
Member

I'm still familiarising myself with how the tree is built and updated, but from the above discussion I can't see any actionable idea here.

As I work through that part of the dependencies node and run more profiling I'll know more and revisit this.

@davkean
Copy link
Member

davkean commented Nov 29, 2018

When it was done other way tree nodes were collapsing when they were updated and tree looked jumping allthe time after DT builds.

I would expect that top level nodes should come from evaluation, design-time build data should only be graph nodes.

@Pilchie
Copy link
Member

Pilchie commented Nov 29, 2018

I think @nguerrera's hope and our plan before was that we would populate non-top level nodes directly from parsing the assets file instead of relying on MSBuild raising items for everyting. That would make design time builds faster, allow us to cache less design time build data, etc.

@drewnoakes drewnoakes modified the milestones: 16.0, 16.1 Jan 15, 2019
@drewnoakes drewnoakes added Triage-Approved Reviewed and prioritized and removed Triage-Approved Reviewed and prioritized labels Jan 15, 2019
@jjmew jjmew modified the milestones: 16.1, 16.X Apr 2, 2019
@panopticoncentral panopticoncentral added the Performance-Scenario-Solution-Open This issue affects solution open performance. label Mar 16, 2020
@drewnoakes drewnoakes removed this from the 16.x milestone May 12, 2020
@drewnoakes drewnoakes added this to the 16.8 milestone May 12, 2020
@drewnoakes drewnoakes modified the milestones: 16.8, 16.9 Jul 24, 2020
@drewnoakes drewnoakes modified the milestones: 16.9, 16.x Nov 23, 2020
@panopticoncentral panopticoncentral added the Priority:2 Work that is important, but not critical for the release label Mar 22, 2021
@drewnoakes drewnoakes modified the milestones: 16.x, 17.x Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature-Dependency-Node "Dependencies" node in Solution Explorer that display project, binary & package references Performance-Scenario-Solution-Open This issue affects solution open performance. Priority:2 Work that is important, but not critical for the release Tenet-Performance This issue affects the "Performance" tenet. Triage-Approved Reviewed and prioritized
Projects
None yet
Development

No branches or pull requests

8 participants