Skip to content

Mark Marvin.ComputeHash32 as noinlining#118851

Merged
AndyAyersMS merged 1 commit intodotnet:mainfrom
AndyAyersMS:Fix118739
Aug 18, 2025
Merged

Mark Marvin.ComputeHash32 as noinlining#118851
AndyAyersMS merged 1 commit intodotnet:mainfrom
AndyAyersMS:Fix118739

Conversation

@AndyAyersMS
Copy link
Copy Markdown
Member

Recent changes to the JIT's inliner may lead to this method but not all of its callees being inlined, which is not good for performance.

There is not much benefit to be had by inlining this method, so mark it as noinline.

Fixes (part of) #118739.

Recent changes to the JIT's inliner may lead to this method but not all of its callees
being inlined, which is not good for performance.

There is not much benefit to be had by inlining this method, so mark it as noinline.

Fixes (part of) dotnet#118739.
Copilot AI review requested due to automatic review settings August 18, 2025 17:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a performance issue in the Marvin hash computation by preventing the ComputeHash32 method from being inlined. Recent JIT inliner changes may cause partial inlining of this method without its callees, which degrades performance.

Key Changes

  • Added [MethodImpl(MethodImplOptions.NoInlining)] attribute to Marvin.ComputeHash32 method to prevent inlining and ensure optimal performance

@github-actions github-actions Bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 18, 2025
@AndyAyersMS
Copy link
Copy Markdown
Member Author

Similar in spirit to #118544.

@EgorBo PTAL
cc @dotnet/jit-contrib @jeffhandley

Would like to have this considered for .NET 10.

@AndyAyersMS
Copy link
Copy Markdown
Member Author

@EgorBot --intel --arm --filter System.Xml.Linq.Perf_XElement.CreateElementWithNamespace

@AndyAyersMS AndyAyersMS merged commit 2dada4c into dotnet:main Aug 18, 2025
143 of 145 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants