roslyn
roslyn copied to clipboard
Using Nullable Disabled semantic model to compute semantic classification
Summary:
Using new API (#70609) to get semantic model with nullability analysis disabled
Report
I completed my investigation, what I see is that the changes reduce the overhead on EnsureNullabilityAnalysisPerformedIfNecessary when we are computing semantic classifications
I have 4 cases:
ALWAYS: when nullability analysis enabled ALWAYSDEFAULT(Using new APIs): when nullability analysis is not set (DEFAULT) - Using new APIs to get Nullable Disabled Semantic ModelNEVER: when nullability analysis is disabled NEVERDEFAULT(Original Semantic Model): when nullability analysis is not set (DEFAULT) - Without new APIs, using Original Semantic Model
perfview comparison:
Summary: This confirms when we use nullable disabled semantic model for semantic classification the computation of nullability analysis drops and overall semantic classification computation becomes faster. Note that AddSemanticClassifications in the caller list drops from 4% to ~0%
Case # 2 with new APIs (Using nullable disabled semantic model)
Case # 4 (currently shipped)
stopwatch comparison:
Summary: This shows an average of 372ms reduced to 132ms when we try to compute semantic classification on a large razor file.
Case # 2 (Using nullable disabled semantic model)
Case # 4 (currently shipped)
Done with review pass (commit 17)
It looks like test/CI failures in compiler layer are meaningful. Will have to investigate.
How should we document the experimental diagnostics? Perhaps we should add a markdown doc similar to https://github.com/dotnet/roslyn/blob/main/docs/Breaking%20API%20Changes.md which lists the "currently active experiments", providing context for each diagnostic ID we are using with [Experimental]?
@AlekseyTs Changes under Compilers are ready for another review pass.
#if DEBUG
This change looks suspicious. It looks like AnalyzeBoundNodeNullability doesn't do any validation unless we are in DEBUG, therefore it doesn't feel useful to call it for RELEASE.
Therefore, it looks like, instead of modifying this code, we should bail out at the very beginning for nullable disabled semantic model in RELEASE mode. There is simply no point in doing all that work.
#Closed
Refers to: src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs:1983 in 95fb86a. [](commit_id = 95fb86a0460c119492a43904864fa07fc76857c5, deletion_comment = True)
Done with review pass (commit 22)
Done with review pass (commit 23)
done with pass.