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
Signed-off-by: Austin Burdine <austin@ghost.org>
Collaborator
|
Review requested:
|
nodejs-github-bot
added
build
Contributor
|
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. |
This was referenced Oct 14, 2026
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Backports v8/v8@2444c10 (https://chromium-review.googlesource.com/c/v8/v8/+/8521518, crbug https://issues.chromium.org/issues/570738027) to v26.x, as suggested in #66692 instead of floating it on
main.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.Changes from the upstream commit:
heap.cc,heap.h,gc-tracer.h: applied unchanged.deps/v8/AUTHORS: same line, merged by hand (context drift).test/unittests/heap/gc-tracer-unittest.cc:GCTracer::SampleAllocation()takes 4 counters on V8 14.6 (no external-memory counter), so the two new tests drop that argument; one comment no longer mentions external memory.v8_embedder_stringbumped to-node.40.Verification:
make -j8 teston macOS arm64: all pass (3 CA/TLS tests neededNODE_USE_SYSTEM_CAunset in my shell, unrelated to this change).deps/v8and ranunittests/GCTracerTest.*andunittests/MemoryReducer.*(23/23 pass). With theheap.ccchange reverted,GCTracerTest.LowAllocationRateWithZeroEmbedderAllocationfails (HasLowAllocationRate()is false); with it, both new tests pass.make test-v8suites (mjsunit, cctest, debugger, inspector, message, preparser): 14,996/14,999 pass. The 3 failures (regress-crbug-1041232,-1104608,-1209152) use--multi-mapped-mock-allocator, whosemremapwith old size 0 isn't supported by the x86 emulation the container ran under.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 vs 11 s with an earlier, embedder-only version of this fix.
Disclaimer
The investigation, the upstream CL and this backport were prepared with help from an AI coding assistant. The fix was reviewed and approved upstream by the V8 heap team; I reviewed the backport diff, and the verification above was run locally on this branch.