[release/9.0-staging] [mono] Switch generic instance cache back to GHashTable; improve ginst hash function#113316
Merged
steveisok merged 2 commits intorelease/9.0-stagingfrom Mar 10, 2025
Conversation
ghost
added
the
steveisok
requested review from
kotlarmilos and
vitek-karas
and removed request for
lambdageek
jeffschwMSFT
approved these changes
Mar 10, 2025
Member
jeffschwMSFT
left a comment
jeffschwMSFT
left a comment
There was a problem hiding this comment.
lgtm. please get a code review. we will take for consideration in 9.0.x
This was referenced Mar 10, 2025
Open
kotlarmilos
approved these changes
Mar 10, 2025
vitek-karas
approved these changes
Mar 10, 2025
Contributor
|
@kg @vitek-karas @jeffschwMSFT Today is code complete for the April Release. If you want this fix included in this release, please get it approved by Tactics and merge it before 4pm PT. |
lewing
approved these changes
Mar 10, 2025
steveisok
added
Servicing-approved
Member
|
Approved by tactics over email |
3 tasks
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
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.
Backport of #113287 to release/9.0-staging
/cc @kg
Customer Impact
Multiple customers have reported seeing significantly slower iOS/MacCatalyst AOT compilation times in .NET 9 compared to .NET 8. We found the slowdown was specific to ARM macs and was due to ARM specific inefficiencies with a hashtable we introduced early on in the .NET 9 cycle.
AOT build times on Apple hardware regressed significantly in .NET 9 and 10, i.e. from ~50 seconds to ~250 seconds.
This change will revert to the hashtable container used for the generic instance cache in .NET 8.0 to address a performance regression introduced by changing to a different container in 9. Also improves the hash function used for the cache (the existing one was suboptimal.)
Regression
Can be traced back to #102039
Testing
Manual benchmarks were performed by @steveisok and @kotlarmilos on Apple hardware. I also performed measurements on Ampere ARM64 and AMD x64 hardware to verify that the performance characteristics of the alternative container used here are acceptable.
Running the test @rolfbjarne used in #110406 (comment) shows we get back to the expected range:
Risk
Low. We used this container in .NET 8 without issue. May cause a slight startup hit for the WASM target specifically since that motivated the original change, but profiling data showed that at most this container accounted for 3% of CPU time in the startup profile we were targeting, so any regression should be in the neighborhood of 5ms or less.
IMPORTANT: If this backport is for a servicing release, please verify that:
release/X.0-staging, notrelease/X.0.Package authoring no longer needed in .NET 9
IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.