Importing Microsoft.CodeAnalysis.targets costs ~3.5% of evaluation time of solution-wide design-time buld

[davkean]

See: dotnet/sdk#1586

3.5% of evaluation time (0.2% of just evaluating the import statement itself) is spent evaluating Microsoft.CodeAnalysis.targets inclusively:

Pass	File	Line #	Expression	Inc (ms)	Inc (%)	Exc (ms)	Exc (%)	#	Bug
Total Evaluation				6159	100%	54	0.9%
Properties (Pass 1)				3312	53.8%	9	0.1%
ItemDefinitionGroup (Pass 2)				14	0.2%	2	0%
Items (Pass 3)				566	9.2%	24	0.4%
Lazy Items (Pass 3.1)				1826	29.6%	158	2.6%
UsingTasks (Pass 4)				59	1%	59	1%
Targets (Pass 5)				327	5.3%	208	3.4%
Properties (Pass 1)	Microsoft.CodeAnalysis.targets	5774	<Import Project="$(CodeAnalysisTargets)" Condition="Exists('$(CodeAnalysisTargets)')" />	215	3.5%	12	0.2%	223

Every single project pays for this import regardless of whether they support or use CodeAnalysis (.NET Core for example, doesn't).

Note: that ~1.3% of this is being tracked by: #22110.

[Pilchie]

Not sure if this is compiler or FxCop targets...

[mavasani]

@tmeschter I know that for the legacy project system there are at least 2 places where we need the default ruleset and ruleset directory being set by this targets file:

1. Code Analysis property page
2. Analyzers node in solution explorer

We do not have the Code Analysis property page in the new project system (correct me if this has changed recently), but we do have the Analyzers dependencies node. Does the latter need the default ruleset? If so, we would still need to import this targets file.

[tmeschter]

@mavasani The Analyzers node shouldn't care about whether or not the CodeAnalysisRuleSet property is defined. Mostly it cares about the settings from the file so we can update the Solution Explorer icons and Properties pane to reflect the effective severity, but it doesn't get those directly from the file. It gets them from the compiler layer, which is responsible for reading the rule set file (if any) and combining it with project-specific settings.

The only time the Analyzers node looks at the file itself is when the user changes a diagnostic severity, but we can't modify the rule set because the project is using one of the defaults. In that case we copy the file to the project, and then apply the modification. I previously hardened this path to handle the possibility that no default rule set was specified, in which case we generate an empty project-local rule set file.

I see two problems with not importing Microsoft.CodeAnalysis.targets, or importing it conditionally:

1. The Code Analysis property page would no longer be able to get the settings it needs.
2. This could be a breaking change for anyone using the Code Analysis rules that have been re-implemented as analyzers, since the default rule set alters the severity for several of those.

[mavasani]

The Code Analysis property page would no longer be able to get the settings it needs.

Do we support the CodeAnalysis property page in the new project system? I thought we did not.

This could be a breaking change for anyone using the Code Analysis rules that have been re-implemented as analyzers, since the default rule set alters the severity for several of those

The default ruleset only affects two scenarios:

1. Legacy FxCop execution - this is explicitly not supported for .NetCore/.NetStandard
2. Live analysis in VS with FxCop analyzers (CA rules defined in the default ruleset) - This experience is already broken for nuget based analyzers as users get different ruleset in command line build (where there is no default ruleset) versus IDE (where we do have the default ruleset). We are already planning on disabling the default ruleset setting for live analysis - users will have to explicitly set a ruleset to override behavior.

[Pilchie]

This experience is already broken for nuget based analyzers as users get different ruleset in command line build (where there is no default ruleset) versus IDE (where we do have the default ruleset)

Why does the commandline not have a default ruleset? I thought that was exactly what these targets provide?

[tmeschter]

The value of the CodeAnalysisRuleSet property should not vary between design-time and full builds. That is a bug.

[mavasani]

@Pilchie Yes, you are correct. I got confused that this targets was imported only in VS, but that is not the case. However, with the FxCop analyzers, now we have an option of packaging the default ruleset(s) along with the VSIX/NuGet and set the default in props.

[tmeschter]

However, with the FxCop analyzers, now we have an option of packaging the default ruleset(s) along with the VSIX/NuGet and set the default in props.

Can you expand on this? When we implemented analyzers we discussed this and decided that VSIX/NuGet packages should never include a .ruleset file because there is no way to compose them. Since they're specified through an MSBuild property only one can win, and the others will be ignored. Default severity should be set in the diagnostic itself, or if for some reason that can't be done, then by updating the <WarnAsError> property in a .props shipped in the VSIX/NuGet.

[mavasani]

@tmeschter - yes we would need the ruleset composition feature for it.