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.

Sign up for GitHub

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

Jump to bottom

Rewrite event registration token logic to improve performance, trimming #1448

Merged
merged 11 commits into from
Jan 18, 2024
Merged

Rewrite event registration token logic to improve performance, trimming #1448

merged 11 commits into from
Jan 18, 2024

Conversation

Sergio0694
Copy link
Member

@Sergio0694 Sergio0694 commented Jan 16, 2024 •
edited
Loading

This PR rewrites the logic to compute event registration tokens, see: dotnet/runtime#96947.
This is both faster, and allows NativeAOT to trim signficantly more stuff, as it makes all delegate targets non-reflectable.

@Sergio0694 Sergio0694 added performance Related to performance work trimming AOT labels Jan 16, 2024
@Sergio0694 Sergio0694 mentioned this pull request Jan 16, 2024
Closed
jkotas
jkotas approved these changes Jan 17, 2024
View reviewed changes
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.

LGTM

@Sergio0694
Copy link
Member Author

@jkotas thank you for all the help and for your time to review the PR, I appreciate it! 🙂

@Sergio0694 Sergio0694 linked an issue Jan 17, 2024 that may be closed by this pull request
Remove Delegate.Method use in EventRegistrationTokenTable<T> #1444
Closed
Base automatically changed from user/sergiopedri/tweak-event-tokens to staging/AOT January 17, 2024 11:36
@Sergio0694 Sergio0694 force-pushed the user/sergiopedri/optimize-event-table branch from 86196b8 to b09e7e4 Compare January 17, 2024 11:40
@Sergio0694
Copy link
Member Author

Saves ~350 KB on .NET 9!! 🎉
I expect the delta to possibly be less than this once #1429 and #1437 are merged, but still, this is awesome!

image

(sizoscope only accounts for ~260 KB, but the .dll size diff is 4441 KB -> 4093 KB)

@Sergio0694 Sergio0694 merged commit 0e1d586 into staging/AOT Jan 18, 2024
9 checks passed
@Sergio0694 Sergio0694 deleted the user/sergiopedri/optimize-event-table branch January 18, 2024 00:40
@Sergio0694 Sergio0694 mentioned this pull request Jan 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Joe4evr Joe4evr Joe4evr left review comments

@jkotas jkotas jkotas approved these changes

@manodasanW manodasanW manodasanW approved these changes

@jkoritzinsky jkoritzinsky Awaiting requested review from jkoritzinsky

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky

Assignees
No one assigned
Labels
AOT performance Related to performance work trimming
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove Delegate.Method use in EventRegistrationTokenTable<T>
4 participants
@Sergio0694 @Joe4evr @manodasanW @jkotas