Move ILLink.LinkAttributes to shared and support NullabilityInfoContext with trimming nullability attributes #56475
Conversation
…eclr - Move IntrinsicAttribute to mono only - Introduce feature switches for NullabilityInfoContext and AggressiveAttributeTrimming - Minor other cleanup
…abilityInfoContext.
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr Issue DetailsThis addresses how we are trimming attributes in .NET 6. The problems being solved here are:
Note: It is probably easiest to review commit-by-commit. The 1st commit is a straight copy from mono to shared, while changes are made in the 2nd commit. @jkotas @MichalStrehovsky @davidwrighton - are there other attributes besides
|
| | MetadataUpdaterSupport | System.Reflection.Metadata.MetadataUpdater.IsSupported | Metadata update related code to be trimmed when set to false | | ||
| | _EnableConsumingManagedCodeFromNativeHosting | System.Runtime.InteropServices.EnableConsumingManagedCodeFromNativeHosting | Getting a managed function from native hosting is disabled when set to false and related functionality can be trimmed. | | ||
| | NullabilityInfoContextSupport | System.Reflection.NullabilityInfoContext.IsSupported | Nullable attributes can be trimmed when set to false | | ||
| | _AggressiveAttributeTrimming | System.AggressiveAttributeTrimming | When set to true, aggressively trims attributes to allow for the most size savings possible, even if it could result in runtime behavior changes | |
There was a problem hiding this comment.
Why does the name have the _ prefix?
There was a problem hiding this comment.
The same reason _EnableConsumingManagedCodeFromNativeHosting has a _ prefix. This isn't intended to be a "public" setting, but only exists for Blazor WASM to trim these attributes, and an escape hatch for developers in case we got it wrong.
buyaa-n
left a comment
There was a problem hiding this comment.
NullabilityInfoContext related changes LGTM
vitek-karas
left a comment
There was a problem hiding this comment.
Looks good - except for the selection of which attributes are dangerous to trim, I'll leave that to more knowledgeable people.
src/libraries/System.Runtime/tests/System/Reflection/NullabilityInfoContextTests.cs
Show resolved
Hide resolved
|
Adding a few more coreclr devs to answer this question: are there other attributes besides cc @JulieLeeMSFT @AndyAyersMS @tommcdon @agocke It looks like runtime/src/coreclr/vm/methodtable.cpp Lines 9849 to 9863 in 3ce1168 |
|
I would be leery of trimming any custom attribute that the runtime checks for. I don't know how to effectively enumerate that set, perhaps @jkotas or @davidwrighton can say... |
Would this be a decent approach? |
Maybe? I don't how easy it actually is to enumerate all the attributes. One place to look is https://github.com/dotnet/runtime/blob/main/src/coreclr/vm/wellknownattributes.h but I suspect it's just convention that attributes are listed there and there may be others that the runtime looks for. |
|
My thought was just to do a |
|
My look at this list found only |
|
After searching for each attribute in the list in
|
This addresses how we are trimming attributes in .NET 6. The problems being solved here are:
NullabilityInfoContext.Createwill throw an exception.Note: It is probably easiest to review commit-by-commit. The 1st commit is a straight copy from mono to shared, while changes are made in the 2nd commit.
Fix #48217
Fix #55860
@jkotas @MichalStrehovsky @davidwrighton - are there other attributes besides
IntrinsicAttributein this list that shouldn't be removed incoreclr? For example, questionable ones for me were:NonVersionableAttribute,AsyncMethodBuilderAttribute,SkipLocalsInitAttribute,EmbeddedAttribute,NativeIntegerAttribute, orIsUnmanagedAttribute?runtime/src/mono/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.xml
Lines 4 to 191 in 6789379