Skip to content

Conversation

@MichalStrehovsky
Copy link
Member

Contributes to #91704.

When we deleted reflection blocking in #85810 we had to update EETypeNode/ConstructedEETypeNode to ensure any MethodTable gets metadata (constructed or not). This was needed because of how reflection was structured - we couldn't create a RuntimeType without knowing about the metadata. After the refactor in #93440, metadata is no longer a prerequisite to constructing a RuntimeType.

The compiler can go back to optimizing away the metadata. This affects things like typeof(Foo) == bar.GetType(). No metadata is needed for this (and we do optimized the Foo MethodTable to unconstructed one) but we still had to generate metadata for reflection stack sake.

Besides the rollback of EEType to the previous shape, this also has a bugfix for #91988 that was found later - interface types used in cast/dispatch should be considered constructed.

I'm seeing 0.1 - 0.7% size saving. Considering interfaces constructed doesn't result in pretty much any regressions and kind of makes sense (they don't have vtables and their interface list is not much cost).

Cc @dotnet/ilc-contrib

Contributes to dotnet#91704.

When we deleted reflection blocking in dotnet#85810 we had to update `EETypeNode`/`ConstructedEETypeNode` to ensure any `MethodTable` also has metadata (constructed or not). This was needed because of how reflection was structured - we couldn't create a `RuntimeType` without knowing about the metadata. After the refactor in dotnet#93440, metadata is no longer a prerequisite to constructing a `RuntimeType`.

The compiler can go back to optimizing away the metadata. This affects things like `typeof(Foo) == bar.GetType()`. No metadata is needed for this (and we do optimized the `Foo` MethodTable to unconstructed one) but we still had to generate metadata.

Besides the rollback of EEType to the previous shape, this also has a bugfix for dotnet#91988 that was found later - interface types used in cast/dispatch should be considered constructed.

I'm seeing 0.1 - 0.7% size saving.
@ghost
Copy link

ghost commented Nov 2, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Contributes to #91704.

When we deleted reflection blocking in #85810 we had to update EETypeNode/ConstructedEETypeNode to ensure any MethodTable gets metadata (constructed or not). This was needed because of how reflection was structured - we couldn't create a RuntimeType without knowing about the metadata. After the refactor in #93440, metadata is no longer a prerequisite to constructing a RuntimeType.

The compiler can go back to optimizing away the metadata. This affects things like typeof(Foo) == bar.GetType(). No metadata is needed for this (and we do optimized the Foo MethodTable to unconstructed one) but we still had to generate metadata for reflection stack sake.

Besides the rollback of EEType to the previous shape, this also has a bugfix for #91988 that was found later - interface types used in cast/dispatch should be considered constructed.

I'm seeing 0.1 - 0.7% size saving. Considering interfaces constructed doesn't result in pretty much any regressions and kind of makes sense (they don't have vtables and their interface list is not much cost).

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@jkotas
Copy link
Member

jkotas commented Nov 2, 2023

Asserts in checked build

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MichalStrehovsky MichalStrehovsky merged commit 1ddb513 into dotnet:main Nov 6, 2023
@MichalStrehovsky MichalStrehovsky deleted the nounconstructed branch November 6, 2023 22:16
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants