Skip to content

Optimize the case when we have a generator not producing output#52979

Merged
jasonmalinowski merged 1 commit intodotnet:release/dev16.10from
jasonmalinowski:fix-generator-assert
Apr 29, 2021
Merged

Optimize the case when we have a generator not producing output#52979
jasonmalinowski merged 1 commit intodotnet:release/dev16.10from
jasonmalinowski:fix-generator-assert

Conversation

@jasonmalinowski
Copy link
Member

@jasonmalinowski jasonmalinowski commented Apr 27, 2021

In 63a1ad6 we made some performance optimizations where we would try to reuse compilations better. The core idea was if a change to a file happens, we have to update the compilation that doesn't contain source generated files so we can re-run generators; if the generated output was the same then we should also do the same updates to the compilation that has source generators so we can better use compiler caches.

The one case that wasn't handled well was if the compilation with and without generated output was actually the same: if a generator was added that isn't producing documents, then those compilations can be the same instance, but we were applying the transformations twice as if they were different. This fired an assert elsewhere in the CompilationTracker that when we're storing compilations away at the very end we shouldn't have two separate but semantically identical compilations if there are no actual generated files anywhere.

The fix taken here is to adjust our logic where we update compilations for changes to regular files: there we were updating both compilations at once. If the compilations are identical, we just null out the "withGenerators" since we won't need to update that as well. That way we're only updating one copy, which will be the one passed to generators again. And if no generated output is still produced, that'll be the single compilation again stored in the FinalState.

@jasonmalinowski jasonmalinowski requested a review from a team as a code owner April 27, 2021 23:03
@ghost ghost added the Area-IDE label Apr 27, 2021
In 63a1ad6 we made some performance
optimizations where we would try to reuse compilations better. The core
idea was if a change to a file happens, we have to update the
compilation that doesn't contain source generated files so we can
re-run generators; if the generated output was the same then we should
also do the same updates to the compilation that _has_ source generators
so we can better use compiler caches.

The one case that wasn't handled well was if the compilation with
and without generated output was actually the same: if a generator was
added that isn't producing documents, then those compilations can
be the same instance, but we were applying the transformations
twice as if they were different. This fired an assert elsewhere in
the CompilationTracker that when we're storing compilations away at
the very end we shouldn't have two separate but semantically identical
compilations if there are no actual generated files anywhere.

The fix taken here is to adjust our logic where we update compilations
for changes to regular files: there we were updating both compilations
at once. If the compilations are identical, we just null out the
"withGenerators" since we won't need to update that as well. That way
we're only updating one copy, which will be the one passed to generators
again. And if no generated output is still produced, that'll be the
single compilation again stored in the FinalState.
@jasonmalinowski jasonmalinowski self-assigned this Apr 29, 2021
@jasonmalinowski jasonmalinowski merged commit caa91bd into dotnet:release/dev16.10 Apr 29, 2021
@jasonmalinowski jasonmalinowski deleted the fix-generator-assert branch April 29, 2021 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants