Allow RazorProjectEngine.Process to be cancelled #11334
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These two argument null checks in private protected methods are already handled elsewhere.
It turns out that nothing calls these APIs with a null file kind, so let's not annotate it that way.
There's nullability analysis all the way through that ensures that the arguments passed to this ctor can't be null.
- Don't expose CreateCodeDocumentCore with optional arguments - Remove unneeded ArgHelper.ThrowIfNull check
Add simple helper class to cache IRazorEngineFeatures and IRazorProjectEngineFeatures by type.
This property isn't necessary. Nearly every caller can be changed to go through RazorProjectEngine.Engine.GetFeatures<T>().
Now that RazorProjectEngine.EngineFeatures is gone, this property can be safely renamed.
Avoid an unnecessary array creation/copy by passing the PooledArrayBuilder<RazorProjectItem> into RazorProjectEngine.GetImportSourceDocuments
- Add NotNull method - Add interpolated string handlers
Because this change modifies the AssumeNotNulll extension methods to defer to Assumed.NonNull, it's necessary to update TelemetryReporter to include the ThrowHelper and Assumed classes when performing its stack walk for faults. I suspect that these types should have already been included anyway, to improve the fault bucketing. It might be useful to add ArgHelper as well, but that's not really in scope here.
This is the same as the previous commit but results in more affected downstream code due to the changes to IRazorEngineFeature. For simplicity, each IRazorEngineFeature implementation now inherits from RazorEngineFeatureBase.
This is similar to the last commit. All IRazorProjectEngineFeature implementations now inherit from RazorProjectEngineFeatureBase.
SourceGeneratorProjectEngine inherited from RazorProjectEngine, but it really didn't need to.
alexgav
approved these changes
Dec 27, 2024
jjonescz
approved these changes
Dec 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We see a lot of memory churn in Razor when opening a solution. There are several contributors to this problem, but one particular issue is that the compiler continues to process files, even if the need for the resulting RazorCodeDocument as passed. Once a
RazorProjectEngine.Process*method has been called, it can't be cancelled, which leads to lots of unnecessary allocations. This pull request lays the groundwork to allow the Razor compiler to be cancelled at an API level. In addition, while working on RazorProjectEngine and friends, I added caching to avoid the same queries running over project engine features over and over again.I recommend reviewing commit-by-commit.
VS insertion PR: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/600134