Make the ILLink benchmark much more realistic

[vitek-karas]

Previously we ran with copy default action (or the equivalence of that) - which is not the default for console/ASP.NET apps

• Didn't test the setup which most people will experience
• ILLink reported many warnings because the framework assumes certain code to be removed by it (due to feature switches and such), but that didn't always kick in due to the copy action - default should be no warnings
• One upside of the previous setting was that the tests stressed the linker much more (especially the MarkStep) and thus the resulting duration is much more precise measuring marking performance as compared to the rest of the linker - effectively it emulated a much larger application.

Fixed a wrong singlewarn- command line.
Fixed feature switched.
Updated feature switches to match the .NET 8 default for command line apps.

Marking this as a draft first since there's some discussion to be had:

• Should we rename the benchmark name? So that history data is not "polluted"? https://github.com/dotnet/performance/blob/main/docs/microbenchmark-design-guidelines.md#benchmarks-are-immutable
• Should we keep the existing one and just introduce a new one which is more realistic?
• Should we change the msbuild variation as well? It currently uses TrimMode=partial.

Future considerations:

• Add another benchmark for ASP.NET AOT project - as that is a good sample of a more complex app
• Add another benchmark with mono, MAUI or Blazor project template - as that is where illink is used heavily and with different settings

/cc Youssef1313

[jtschuster]

Should we rename the benchmark name? So that history data is not "polluted"?
Should we keep the existing one and just introduce a new one which is more realistic?

I'm in favor of keeping both. While the previous benchmark wasn't a realistic situation, it did have to analyze all the framework libraries, which covered more analysis scenarios and makes it useful for overall performance benchmarking of the linker. Since this is just a HelloWorld app, we're rooting a lot less and analyzing much less if we use 'trim'. I think both are valid scenarios to test and offer different insights into the performance.

Should we change the msbuild variation as well? It currently uses TrimMode=partial.

Do you think we could remove that? The only benefit I could see from that test is that the arguments to the linker will be updated as we update the SDK.

[sbomer]

Agreed, I think we should keep both, at least until we have a more realistic benchmark that stresses the linker more than a helloworld with the default trimming settings.

I think the TrimMode partial could be removed. It looks to me like it was supposed to emulate the copy behavior (see #3126) in the MSBuild benchmark, but I don't think it accomplished what it intended because the framework assemblies should be getting trimmed with partial. I'm not sure the MSBuild benchmark provides much value, so maybe we could remove that entire benchmark.

[vitek-karas]

Another idea is to:

• In the benchmark setup, run dotnet publish -p:PublishTrimmed=true /bl, grab the binlog, and using structured log package parse it and find the command line used to run the illink. Then run that in the benchmark.
• This would have two advantages:
  • No need to maintain the command line compatible and representative of the current version
  • No need for code to grab list of framework assemblies, prepare a temp folder and so on - it would happen in the normal output directory
• Disadvantage is the dependency on msbuild's binlog and especially the package to parse it

[jtschuster]

The LinkTask is a ToolTask, which looks like it has some functionality to create a response file. Maybe we could have an option to write it to disk permanently and use that in this benchmark? That would have been helpful for debugging some issues in the past, too.

[sbomer]

Agreed, in fact I like the way ILCompiler does this (it always leaves the .rsp file in the intermediate directory). Relevant: dotnet/msbuild#4315. Maybe we should work around that and write the response file ourselves.

[vitek-karas]

Good point - let's solve the "repro" problem for the product first and then we can use it in the benchmarks.
Sven - how much do you think there's risk in switching the illink invocation from normal command line, to rsp file? It doesn't feel dangerous at all - especially if we do it for 9 only.

[sbomer]

I don't see much risk in using a response file - we technically already do, but it's created in a temp directory and deleted each time by ToolTask:
https://github.com/dotnet/msbuild/blob/0125fc9fb838951c8750135c71336aa1f5e868a9/src/Utilities/ToolTask.cs#L543-L567.
It'd be nice to fix this in ToolTask (dotnet/msbuild#4315).

[jtschuster]

If we don't want to wait for msbuild, we could also override ExecuteTool and have a switch to just output the rsp file somewhere and skip actually running the linker. Then we wouldn't need to run the whole linker before doing it again in the benchmark.

Or we could create a whole new task which just outputs the rsp file somewhere and shares code with the link task.