Address the performance issues for bulk suppression in IDE. #6096
Conversation
mavasani
added
Area-Analyzers
Tenet-Performance
mavasani
force-pushed
the
BulkSuppressPerfFixes
branch
from
572bfed to
489d4d8
Compare
|
@dotnet-bot retest lin please |
|
@dotnet-bot retest linux please |
mavasani
force-pushed
the
BulkSuppressPerfFixes
branch
2 times, most recently
from
e6f7e34 to
a2f3403
Compare
|
@dotnet-bot retest eta please |
|
@heejaechang @srivatsn @basoundr can you please review? |
|
Approved pending reviews. |
|
@heejaechang @srivatsn @natidea can you please take a look? |
| @@ -110,8 +111,7 @@ protected override SyntaxNode AddGlobalSuppressMessageAttribute(SyntaxNode newRo | |||
| bool needsLeadingEndOfLine) | |||
| { | |||
| var attributeArguments = CreateAttributeArguments(targetSymbol, diagnostic); | |||
| var attribute = SyntaxFactory.Attribute(SyntaxFactory.ParseName(SuppressMessageAttributeName), attributeArguments) | |||
| .WithAdditionalAnnotations(Simplifier.Annotation); | |||
| var attribute = SyntaxFactory.Attribute(SyntaxFactory.ParseName(SuppressMessageAttributeName), attributeArguments); | |||
There was a problem hiding this comment.
how about doing minimal type name here manually as well like you did for formatting?
There was a problem hiding this comment.
What API can I use to get the minimal type name? SuppressMessageAttributeName = System.Diagnostics.CodeAnalysis.SuppressMessage.
There was a problem hiding this comment.
get symbol for suppress attribute (using GetSymbolBymetadata ...) and symbol has ToMinimallyQualifed ... method in it.
also, for global suppression, since you are putting all those attribute to same context, get minimallyQualifed string once, and reuse them.
|
I dont think #1 will work unless we block vs and do explicit build + suppression and then unblock vs. and make sure the suppression from build error in the error list is disabled. |
|
"I dont think #1 will work unless we block vs and do explicit build + suppression and then unblock vs. and make sure the suppression from build error in the error list is disabled." We already block the user on doing any operations while suppression is being computed and applied - we do so by bringing up a cancellable wait dialog. The user still has the option to modify sources while the build is going on - however, this only means the explicit build snapshot against which suppression was executed matches the snapshot on which user explicitly invoked this command, not the latest live snapshot. This is by design and no different from user waiting till build + suppression command finished and then modifying sources - you are likely to have the suppressions get out of date and stale as you change your top level definitions, and the diagnostics on the original method symbol will start showing up as active again and you need to generate new suppressions. My point is the command "Suppress active issues" guarantees generating suppressions on the solution snapshot when the command was invoked, but not the live - and that is by design. |
|
I agree with "make sure the suppression from build error in the error list is disabled." @srivatsn - do you agree? We will use build diagnostics for suppression only during explicit "Suppress active issues" command from solution explorer, but not support suppression state/error list suppression for build diagnostics. |
|
Yes I agree. |
mavasani
force-pushed
the
BulkSuppressPerfFixes
branch
from
a2f3403 to
7c3d080
Compare
|
@heejaechang I have updated the PR, can you please take a look? |
mavasani
force-pushed
the
BulkSuppressPerfFixes
branch
from
7c3d080 to
aa26ad9
Compare
|
👍 |
1. "Suppress active issues" command from solution explorer was doing couple of builds - one command line build to compute the FxCop diagnostics and another intellisense build to fetch Roslyn diagnostics. This change removes the second build and instead uses the diagnostics emitted from the build for suppression. 2. Bulk suppressing multiple diagnostics from error list/suppress active issues command was spending more than 50% of time in formatting the generated global suppressions file. This delay grows significantly larger with 1000+ issues being suppressed. This fix changes us to not format the entire suppressions file at once, but instead format each attribute separately and avoid formatting the batched file. This significantly brings down the formatting time. 3. Thread in the cancellation token during batch fix computation - otherwise cancelling the wait dialog while computing suppressions fix will make VS unresponsive. Fixes dotnet#6066
mavasani
force-pushed
the
BulkSuppressPerfFixes
branch
from
aa26ad9 to
1866409
Compare
|
@dotnet-bot test prtest/win/dbg/unit32 |
|
@dotnet-bot retest prtest/win/dbg/unit32 please |
Address the performance issues for bulk suppression in IDE. 1. "Suppress active issues" command from solution explorer was doing couple of builds - one command line build to compute the FxCop diagnostics and another intellisense build to fetch Roslyn diagnostics. This change removes the second build and instead uses the diagnostics emitted from the build for suppression. 2. Bulk suppressing multiple diagnostics from error list/suppress active issues command was spending more than 50% of time in formatting the generated global suppressions file. This delay grows significantly larger with 1000+ issues being suppressed. This fix changes us to not format the entire suppressions file at once, but instead format each attribute separately and avoid formatting the batched file. This significantly brings down the formatting time. 3. Thread in the cancellation token during batch fix computation - otherwise cancelling the wait dialog while computing suppressions fix will make VS unresponsive. Fixes #6066
Fixes #6066