Move AspNetCoreBenchmarkAttribute to shared BenchmarkRunner.Sources

[gfoidl]

Background / Motivation

BenchmarkRunner.Sources contains common code for easily setting up benchmarks:

<ItemGroup>
 <Compile Include="..\..\shared\Microsoft.AspNetCore.BenchmarkRunner.Sources\**\*.cs">
   <Link>Shared\%(FileName)%(Extension)</Link>
 </Compile>
</ItemGroup>

So the benchmark-project just needs to add the intrinsically benchmark-code, for instance Microsoft.Extensions.Primitives.Performance.

Quite easy -- one may think...
When running the benchmarks I always get trapped by No loggers defined, you will not see any progress!.
It's because there needs to be added the AspNetCoreBenchmark at assembly-level (via AssemblyInfo.cs).

Proposal

To make the "setup" for benchmark-projects consistent with e.g. Program.cs this attribute should be moved to the shared sources, so no manual adding / copying of AssemblyInfo.cs is needed.

In aspnet/SignalR#1251 (comment) something similar was mentioned.

Affected projects

• Microsoft.Extensions.Primitives.Performance
• Microsoft.Extensions.ObjectPool.Performance
• Kestrel.Performance
• Microsoft.AspNetCore.SignalR.Microbenchmarks
• Microsoft.AspNetCore.Razor.Performance
• DI.Performance
• Microsoft.AspNetCore.Mvc.Performance

[gfoidl]

/cc @pakrym

[Eilon]

@sebastienros / @mikeharder / @davidfowl - thoughts on this? Worth the effort?

[gfoidl]

I can submit a PR when this proposal is accepted

[pakrym]

@Eilon this one is about microbenchmarks.

The reason we don't bring assembly attribute by default is that it would be impossible to override it globally with different configuration type.
I don't think adding an attribute when creating a project is too much of a hassle especially considering this package is not intended to be used outside ASP.NET Core repositories.

[gfoidl]

to override it globally with different configuration type.

This makes sense. But so far there is the same setup in all benchmark repos within ASP.NET Core.
That's why I would add the attribute to the "default-setup".
In case of a different configuration one can "opt-out" (i.e. don't include the linked file).

is too much of a hassle

Of course not, but it's very easy to forget (because all other stuff for running benchmarks is included by default) and seems strange that this attribute is missing.

[pakrym]

In case of a different configuration one can "opt-out" (i.e. don't include the linked file).

How do you opt-out of a single file out of multiple brought by a nuget package?

[gfoidl]

Ups, I didn't realize that this is shipped by nuget to the other repos. So the opt-out is bullshit.

So your point

to override it globally with different configuration type.

is a strong argument for not changing this behavior. I'm closing this issue. Thanks for clarification!