-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Avoid running generators when nothing has changed in a compilation tracker. #73897
Conversation
// This flag can then be used later when we hear about external user events (like save/build) to | ||
// decide if we need to do anything. If the generated documents are up to date, then we don't need | ||
// to do anything in that case. | ||
var generatedDocumentsUpToDate = creationPolicy.GeneratedDocumentCreationPolicy == GeneratedDocumentCreationPolicy.Create; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: looking at the code below this line is very relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
importantly, we only generate the docs when the policy is GeneratedDocumentCreationPolicy.Create
, but in balanced mode we then immediately transition after this line to .DoNotCreate mode.
So the final state will say "i just generated up to date docs, but any forks off of me should reuse the docs and not regenerate them". Those forks will then end up back in the FinalCompilationTrackerState, except this time they will say "i did not just generate the SG docs" as their creation policy on this line was DoNotCreate.
src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.CompilationTracker.cs
Outdated
Show resolved
Hide resolved
…ationState.CompilationTracker.cs
// produced, then clearly we don't need to do anything. Nothing changed between then and now, so we | ||
// can reuse the final compilation as is. | ||
if (state is FinalCompilationTrackerState { GeneratedDocumentsUpToDate: true }) | ||
return this; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here's the checking point. We have been told that there was an external signal to rerun generators (a save or build) and that it was not the user explicitly clicking on the button that says "dump all generator state and rerun". Since we can see we just did that, we don't need to proceed.
Noticed this when i was wondering my no-op builds and ocd saving was causing one of my generators to rerun while under the debugging. |
@jasonmalinowski @ToddGrun ptal. |
The 'balanced' mode for SGs ensures that if we hear about a save of a document in a project, or a build in that project, that we then mark the project as wanting its generators rerun. This is good in the case where the project has actually changed, and thus asking generators to run will produce a potentially new/up-to-date result. However, this is pointless when the project is fully up to date (like it just ran generators already on its latest contents) and running generators again would not do anything.
This is beneficial as there's no actual mechanism that prevents us from hearing about saves/builds that don't actually change the content of a project. For example, you can hit 'save' in an already saved file. And a full build will still notify us about all the projects that were 'built' even if the fast-up-to-date-check ended up skipping them. Forcing the workspace to dump the cached compilations for these projects is wasteful.
Note: a change to Project A that is depended upon by Project B by a save/build will still cause Project B to be regenerated. This is because the change to A will automatically go and clear the info for downstream projects as well.
The only change here is when we see an uncleared project that just computed its own generated docs.