Skip to content

Avoid inlining Random.InternalSample - #131714

Merged
AndyAyersMS merged 1 commit into
dotnet:mainfrom
AndyAyersMS:fix-random-internalsample-inlining
Aug 2, 2026
Merged

Avoid inlining Random.InternalSample#131714
AndyAyersMS merged 1 commit into
dotnet:mainfrom
AndyAyersMS:fix-random-internalsample-inlining

Conversation

@AndyAyersMS

@AndyAyersMS AndyAyersMS commented Aug 2, 2026

Copy link
Copy Markdown
Member

This method contains an inherently unpredictable branch that benefits from if conversion. If inlined into a caller with a loop we lose the if conversion and performance suffers, and there does not appear to be other inlining benefit.

We can reconsider if/when we've freed up if-conversion from its current "not in loop" constraint.

Fixes #117787
Fixes #130359

This method contains an inherently unpredictable branch that benefits
from if conversion. If inlined into a caller with a loop we lose the
if conversion and performance suffers, and there does not appear to
be other inlining benefit.

We can reconsider if/when we've freed up if-conversion from its current
"not in loop" constraint.

Fixes dotnet#117787

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 adds an explicit JIT hint to keep Random’s compatibility PRNG InternalSample as a standalone method, aiming to avoid unfavorable codegen when the method gets inlined into hot loops.

Changes:

  • Add [MethodImpl(MethodImplOptions.NoInlining)] to CompatPrng.InternalSample().
  • Add an explanatory comment documenting why the method should not be inlined.

@AndyAyersMS

Copy link
Copy Markdown
Member Author

Local Windows x64 results:

Benchmark Baseline Changed
#117787, TieredPGO=1 821.4 µs 279.6 µs
#117787, TieredPGO=0 529.2 µs 541.9 µs (same)

All 40 dotnet/performance Perf_Random cases showed no regressions; NextDouble improved 40% and NextSingle 49%.

Also improves performance of #130359

Iterations Baseline NoInlining
1–10 53.1 ms 52.5 ms
11–20 48.3 ms 49.4 ms
21–30 48.7 ms 49.3 ms
31–40 93.0 ms 46.8 ms
41–50 123.3 ms 45.6 ms
51–60 122.8 ms 45.7 ms
61–70 121.3 ms 46.2 ms
71–80 120.1 ms 46.8 ms
81–90 120.2 ms 46.8 ms
91–100 119.7 ms 46.6 ms

@EgorBo PTAL
fyi @dotnet/jit-contrib

@AndyAyersMS
AndyAyersMS requested a review from EgorBo August 2, 2026 16:44
@AndyAyersMS
AndyAyersMS merged commit 2d0ca35 into dotnet:main Aug 2, 2026
144 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-rc1 milestone Aug 3, 2026
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.

[Regression] Significant performance drop in Random.Next() starting from .NET 8 with ReadyToRun enabled Random.Next Tier1 slower than Tier0

4 participants