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

Update static graph docs #5259

Closed
wants to merge 14 commits into from

Conversation

cdmihai
Copy link
Contributor

@cdmihai cdmihai commented Apr 8, 2020

The changes reflects the state of things with #5222's changes. As potential extra changes go into #5222, I'll also update this PR.

More specifically, #5222 implements this region of the docs: https://github.com/microsoft/msbuild/compare/master...cdmihai:cacheAndIsolationDocumentation?expand=1#diff-5de82af4d8f52787d69b38fd58c509caR44

@rainersigwald rainersigwald added this to the MSBuild 16.7 Preview 1 milestone Apr 10, 2020
Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got lost in the 'How isolation exemption complicates everything :(' section, so this is only a partial review. I'll try to get back to it next week if I have time.

documentation/specs/static-graph-implementation-details.md Outdated Show resolved Hide resolved
documentation/specs/static-graph-implementation-details.md Outdated Show resolved Hide resolved
documentation/specs/static-graph-implementation-details.md Outdated Show resolved Hide resolved
documentation/specs/static-graph-implementation-details.md Outdated Show resolved Hide resolved
documentation/specs/static-graph-implementation-details.md Outdated Show resolved Hide resolved
documentation/specs/static-graph-implementation-details.md Outdated Show resolved Hide resolved
documentation/specs/static-graph-implementation-details.md Outdated Show resolved Hide resolved
documentation/specs/static-graph-implementation-details.md Outdated Show resolved Hide resolved
@@ -297,56 +311,50 @@ Because referenced projects and their entry targets are guaranteed to be in the
### Isolated graph builds
When building a graph in isolated mode, the graph is used to traverse and build the projects in the right order, but each individual project is built in isolation. The build result cache will just be in memory exactly as it is today, but on cache miss it will error. This enforces that both the graph and target mappings are complete and correct.

Furthermore, running in this mode enforces that each (project, global properties) pair is executed only once and must execute all targets needed by all projects which reference that node. This gives it a concrete start and end time, which leads to some perf optimizations, like garbage collecting all project state (except the build results) once it finishes building. This can greatly reduce the memory overhead for large builds.
Furthermore, running in this mode enforces that each (project, global properties) pair is executed only once and must execute all targets needed by all projects which reference that node. This gives it a concrete start and end time, which leads to some potential perf optimizations, like garbage collecting all project state (except the build results) once it finishes building. This can greatly reduce the memory overhead for large builds.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like some of this could be replaced with references to the new doc you wrote. (Doing so would make it less likely our docs would stay stale if this changes.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do a pass and try to reduce duplicates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upon looking over it, not much can be reduced. My intention was to keep high level information in this document, and move all code implementation details into the other one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, sounds good. My best idea for how to completely eliminate any duplicate information would also be a nightmare to maintain, so... 😄

documentation/specs/static-graph.md Outdated Show resolved Hide resolved
cdmihai and others added 10 commits May 4, 2020 14:25
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
Co-authored-by: Forgind <Forgind@users.noreply.github.com>
@Forgind Forgind added the Area: Static Graph Issues with -graph, -isolate, and the related APIs. label Jan 15, 2021
@cdmihai cdmihai closed this Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Static Graph Issues with -graph, -isolate, and the related APIs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants