Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop rooting nested types for helper types #1521

Merged
merged 6 commits into from
Mar 22, 2024

Conversation

Sergio0694
Copy link
Member

@Sergio0694 Sergio0694 commented Mar 1, 2024

While looking at sizoscope traces with @jkoritzinsky, he noticed that the linker was rooting a considerable amount of code due to virtual methods on nested types for helper types, ie. all of the vtable types. These vtable types are no longer used at all with new projections (and NAOT will not even attempt to find them at all, and will just fail if using old projections). This PR updates the trimming annotations to stop rooting all of these nested vtable types, making the fallback path using older projections not trim-safe. It will still work though, and we added some explicit annotations to preserve the generic vtable types that are generally needed by applications, only on CoreCLR, so we should be able to maintain back-compat there when trimming is used. This makes the back-compat support pay-for-play and avoid the binary size increase for customers using new projections and NAOT especially.

See:

@dongle-the-gadget
Copy link
Contributor

This would break over existing trimmed users.

@Sergio0694
Copy link
Member Author

Yes, but we could detail that in our release notes, and it would only break users of other 3rd party WinRT components that don't get updated in sync (which combined with the users of C#, and using trimming, makes for a very small number of users). Those users can wait to make sure their dependencies are updated before bumping CsWinRT, or alternatively they can make sure to manually root those projection assemblies as a temporary workaround. But this way we can make this pay for play.

@Sergio0694 Sergio0694 force-pushed the user/sergiopedri/stop-rooting-vtables branch from 5a7592d to a6ba167 Compare March 18, 2024 22:42
@Sergio0694 Sergio0694 force-pushed the user/sergiopedri/stop-rooting-vtables branch from a6ba167 to ca9f2fa Compare March 19, 2024 02:51
@Sergio0694
Copy link
Member Author

Saves 213 KB 🎉

image

@manodasanW
Copy link
Member

Confirmed types don't get trimmed when building with old projections.

@Sergio0694 Sergio0694 merged commit 46bb1b5 into staging/AOT Mar 22, 2024
10 checks passed
@Sergio0694 Sergio0694 deleted the user/sergiopedri/stop-rooting-vtables branch March 22, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants