Fix runtime metrics memory_load calculation - #9031
Conversation
BenchmarksBenchmark execution time: 2026-08-13 10:41:56 Comparing candidate commit 9dbb8eb in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 71 metrics, 0 unstable metrics, 66 known flaky benchmarks, 60 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9031) and master. ✅ No regressions detected |
d481ab3 to
f6f8901
Compare
f6f8901 to
d64fcc6
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d64fcc6aa0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…a 0-100 percentage, handling differences with GC hard limits
d64fcc6 to
9dbb8eb
Compare
Summary of changes
Fixes the runtime metrics value of
runtime.dotnet.gc.memory_loadin containersReason for change
#9015 flagged that the calculation we're currently doing for
memory_loadis incorrect. This metric is meant to flag essentially how close you are to an OOM. In the "new" implementation (which avoids using event pipes to prevent crashes) we read GC values usingGC.GetGCMemoryInfo()which gives some values of interest, e.g.:MemoryLoadBytes- This is the amount of physical memory the process is using (in bytes).TotalAvailableMemoryBytes- This is the amount of physical memory on the machine available for the GC to use. In containers with memory limits, this is set to 75% of the available memory (this is the bug).HighMemoryLoadThresholdBytes- This is the percentage of total physical memory on the machine, being used, at which point the GC becomes more aggressive.Our previous implementation assumed that
TotalAvailableMemoryByteswas... well... the total available memory in bytes, and so we calculated memory_load as a _percentage using (effectively)However, in containers, the value returned by
TotalAvailableMemoryBytesis not the total available memory, it's 75% of that, which leads to our memory load calculation being 1/3 too high, and leads to absurdities like memory load being > 100% at high load. That's becauseMemoryLoadBytesmaxes out at the real total available memory (before OOMing); i.e. it's the "real" total memory that matters, not the "fake" 75% limit that is provided inTotalAvailableMemoryBytes.The real problem is: there's no simple way for us to get the "real" total available memory from the runtime 🙁
Implementation details
I could see essentially two different options:
HighMemoryLoadThresholdBytesto infer the total memory.Both have their difficulties, but the second approach is the one I went for, as ultimately there were fewer moving parts. We can calculate the real available memory from
HighMemoryLoadThresholdBytesgiven thatWhich means we need
HighMemoryLoadThresholdBytesandHighMemoryThresholdPercent. We definitely have the first one, but we also need the threshold. This is calculated in the GC in two steps (this hasn't ever changed in .NET Core AFAICT):DOTNET_GCHighMemPercent/COM_GCHighMemPercentvariables or the"System.GC.HighMemoryPercent"AppContext switchHighMemoryThresholdPercent = 90%TotalProcessorCountAPI that returns the total number of logical processors on the machine #9029)Once we have this threshold, we can then obtain
RealAvailableMemoryusingand from there we can get the memory load:
Test coverage
This is kind of tricky, because it naturally relies on how much memory we're using. One way to test is simply set the config - before we would give wildly incorrect values, now we shouldn't. But the container limits issue is harder to test in CI. Instead, I fell back to manually testing locally for that, and showed that it's working as expected now
Other details
Fixes #9015