Repository navigation
Conversation
Original commit message:
[heap] Treat zero allocation throughput as a low allocation rate
Since https://crrev.com/c/5935313 allocation throughput is tracked with
SmoothedBytesAndDuration, which has no lower bound. A sampled throughput
of exactly 0 -- nothing was allocated, or the smoothed value decayed to
0 after a long idle period -- made ComputeMutatorUtilizationImpl()
return a mutator utilization of 0 ("maximal allocation"). For an isolate
that never allocates on the CppHeap, HasLowAllocationRate() is then
never true and the MemoryReducer cannot start a GC through its
low-allocation condition; only the 100s watchdog can. Node.js hits this
for any process that does not use node:vm.
Remove the special case. A mutator speed of 0 now yields a mutator
utilization of 1, i.e. the lowest possible allocation rate. gc_speed is
either the conservative default or at least 1 byte/ms, so the formula
stays well defined.
Bug: 570738027
Change-Id: I84cd51607df3c31c2f1e02bb2fe258e5506131ce
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8521518
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#110599}
Refs: v8/v8@2444c10
Fixes: nodejs#66564
|
Review requested:
|
nodejs-github-bot
added
build
|
Welcome to Node.js, and thank you for your first contribution! Before review, please take a moment to read:
Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal. Caution AgentScan found account activity patterns that may be consistent with automation. This is a heuristic, not proof that this pull request was opened by an agent or violates policy. AI-assisted contributions are permitted, but automated tooling must not open pull requests without advance approval, and contributors must personally understand, test, verify, and take responsibility for every submitted change. See the AgentScan analysis, AI use policy, and automation policy for additional context. |
2 participants
Cherry-picks v8/v8@2444c10 (https://chromium-review.googlesource.com/c/v8/v8/+/8521518, crbug https://issues.chromium.org/issues/570738027).
Fixes: #66564
Since V8's allocation throughput moved to
SmoothedBytesAndDuration, a throughput of exactly 0 madeComputeMutatorUtilizationImpl()report "maximal allocation". Node never allocates on the CppHeap unlessnode:vmis used, soHeap::HasLowAllocationRate()was never true and the MemoryReducer could only start a GC through its 100 s watchdog. Upstream removed the zero special case, so 0 now counts as the lowest allocation rate (for the young, old and embedder terms).Repro and traces: https://github.com/acburdine/node-memory-reducer-repro. On v24.21.0, time from the end of an allocation burst to the first memory-reducer GC was 100 s / 100 s / 141 s (idle with a logging timer / silent / trickle of garbage) vs 11 s in all three with an earlier, embedder-only version of this fix. Those patched numbers predate the final upstream change, but the repro only exercises the embedder term, which both versions treat the same way.
The upstream commit adds
GCTracerTest.LowAllocationRateWithZeroEmbedderAllocation(fails without the fix) andGCTracerTest.HighAllocationRateWithEmbedderAllocation. V8's CQ also rannode_ci_linux64_relagainst it.v8_embedder_stringbumped to-node.16.Disclaimer
The investigation, the upstream CL and this cherry-pick were prepared with help from an AI coding assistant. The fix was reviewed and approved upstream by the V8 heap team, and the repro numbers above were checked against the traces in the repro repository.