Opt out of metadata trimming #2020
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Benchmark.NET opts itself into trimming, but it's trim compatible only by accident (it's reflecting on things that are statically referenced and therefore would be kept by IL-based trimming solutions). NativeAOT is able to trim metadata more aggressively - e.g. an accessed field might still have its name and metadata removed and the only thing that is kept is a location in memory that can be read and written. I'm now seeing Benchmark.NET failing with latest ILCompiler packages after dotnet/runtime#70201 that enabled more aggressive trimming. Benchmark.NET should ideally `<EnableTrimAnalyzer>true</EnableTrimAnalyzer>` in the BenchmarkDotNet.csproj project and fix all the warnings and the problem will go away. But enabling compat mode should help too.
|
The failure looks like this. Normally this would be some silent weird behavior (because the field just be missing), but BenchmarkDotNet is opting into |
adamsitnik
approved these changes
Jun 22, 2022
MichalStrehovsky
added a commit
to MichalStrehovsky/BenchmarkDotNet
that referenced
this pull request
Jul 20, 2022
Another attempt at fixing the problem I tried to fix in dotnet#2020. In that pull request, I reflection-rooted all of BenchmarkDotNet, but that looks like it increases the size of the closure too much (dotnet/performance#2532). So here I'm rolling that part back and annotating enough of the `Characteristic` APIs to make it so that we shouldn't run into the original problem. I basically added `<EnableTrimAnalyzer>true</EnableTrimAnalyzer>` to the BechmarkDotNet project, looked at the trimming warning in https://github.com/dotnet/BenchmarkDotNet/blob/63e28c100a42a6492d09a0b93a8a4c141061bb0d/src/BenchmarkDotNet/Characteristics/CharacteristicHelper.cs#L49-L68 where we were getting the crash and kept getting annotations until it stopped generating new warnings. We now have an annotation in a spot that should make sure the `Job` class has all fields and properties kept (it's used with one of the annotated `Create` methods). There are more trimming warnings that I didn't try to solve because they're unrelated to the problem at hand.
adamsitnik
pushed a commit
that referenced
this pull request
Jul 20, 2022
* Make a bit of BenchmarkDotNet trimmable Another attempt at fixing the problem I tried to fix in #2020. In that pull request, I reflection-rooted all of BenchmarkDotNet, but that looks like it increases the size of the closure too much (dotnet/performance#2532). So here I'm rolling that part back and annotating enough of the `Characteristic` APIs to make it so that we shouldn't run into the original problem. I basically added `<EnableTrimAnalyzer>true</EnableTrimAnalyzer>` to the BechmarkDotNet project, looked at the trimming warning in https://github.com/dotnet/BenchmarkDotNet/blob/63e28c100a42a6492d09a0b93a8a4c141061bb0d/src/BenchmarkDotNet/Characteristics/CharacteristicHelper.cs#L49-L68 where we were getting the crash and kept getting annotations until it stopped generating new warnings. We now have an annotation in a spot that should make sure the `Job` class has all fields and properties kept (it's used with one of the annotated `Create` methods). There are more trimming warnings that I didn't try to solve because they're unrelated to the problem at hand.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Benchmark.NET opts itself into trimming, but it's trim compatible only by accident
(it's reflecting on things that are statically referenced and therefore would be kept by IL-based trimming solutions). NativeAOT is able to trim metadata more aggressively - e.g. an accessed field might still have its name and metadata removed and the only thing that is kept is a location in memory that can be read and written.I'm now seeing Benchmark.NET failing with latest ILCompiler packages after dotnet/runtime#70201 that enabled more aggressive trimming.It's reflecting on a field that is not used in any other way than by unanalyzable reflection. We would previously keep all fields, irrespective of whether they're used on reflected on dotnet/runtime#70546.
Benchmark.NET should ideally
<EnableTrimAnalyzer>true</EnableTrimAnalyzer>in the BenchmarkDotNet.csproj project and fix all the warnings and the problem will go away. But enabling compat mode should help too.