Skip to content

Commit bb0f0b9

Browse files
committed
deps: V8: backport 2444c109f4f8
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: #66564 Signed-off-by: Austin Burdine <austin@ghost.org>
1 parent 0d10bfe commit bb0f0b9

6 files changed

Lines changed: 42 additions & 4 deletions

File tree

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242

4343
# Reset this number to 0 on major V8 upgrades.
4444
# Increment by one for each non-official patch applied to deps/v8.
45-
'v8_embedder_string': '-node.57',
45+
'v8_embedder_string': '-node.58',
4646

4747
##### V8 defaults for Node.js #####
4848

‎deps/v8/AUTHORS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ Artem Kobzar <artem.kobzar@jetbrains.com>
7676
Arthur Islamov <arthur@islamov.ai>
7777
Asuka Shikina <shikina.asuka@gmail.com>
7878
Aurèle Barrière <aurele.barriere@gmail.com>
79+
Austin Burdine <austin@ghost.org>
7980
Aviv Keller <me@aviv.sh>
8081
Bala Avulapati <bavulapati@gmail.com>
8182
Bangfu Tao <bangfu.tao@samsung.com>

‎deps/v8/src/heap/gc-tracer.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,7 @@ class V8_EXPORT_PRIVATE GCTracer {
608608
FRIEND_TEST(GCTracerTest, BackgroundMajorMCScope);
609609
FRIEND_TEST(GCTracerTest, CyclePriorities);
610610
FRIEND_TEST(GCTracerTest, EmbedderAllocationThroughput);
611+
FRIEND_TEST(GCTracerTest, HighAllocationRateWithEmbedderAllocation);
611612
FRIEND_TEST(GCTracerTest, MultithreadedBackgroundScope);
612613
FRIEND_TEST(GCTracerTest, NewSpaceAllocationThroughput);
613614
FRIEND_TEST(GCTracerTest, PerGenerationAllocationThroughput);
@@ -616,6 +617,7 @@ class V8_EXPORT_PRIVATE GCTracer {
616617
FRIEND_TEST(GCTracerTest, IncrementalMarkingDetails);
617618
FRIEND_TEST(GCTracerTest, IncrementalScope);
618619
FRIEND_TEST(GCTracerTest, IncrementalMarkingSpeed);
620+
FRIEND_TEST(GCTracerTest, LowAllocationRateWithZeroEmbedderAllocation);
619621
FRIEND_TEST(GCTracerTest, MutatorUtilization);
620622
FRIEND_TEST(GCTracerTest, RecordMarkCompactHistograms);
621623
FRIEND_TEST(GCTracerTest, RecordScavengerHistograms);

‎deps/v8/src/heap/heap.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3668,9 +3668,7 @@ namespace {
36683668

36693669
double ComputeMutatorUtilizationImpl(double mutator_speed,
36703670
std::optional<double> gc_speed) {
3671-
constexpr double kMinMutatorUtilization = 0.0;
36723671
constexpr double kConservativeGcSpeedInBytesPerMillisecond = 200000;
3673-
if (mutator_speed == 0) return kMinMutatorUtilization;
36743672
if (!gc_speed) gc_speed = kConservativeGcSpeedInBytesPerMillisecond;
36753673
// Derivation:
36763674
// mutator_utilization = mutator_time / (mutator_time + gc_time)

‎deps/v8/src/heap/heap.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ class Heap final {
642642
// null.
643643
V8_INLINE bool CanSafepoint() const { return deserialization_complete(); }
644644

645-
bool HasLowAllocationRate();
645+
V8_EXPORT_PRIVATE bool HasLowAllocationRate();
646646
bool HasHighFragmentation();
647647

648648
void ActivateMemoryReducerIfNeeded();

‎deps/v8/test/unittests/heap/gc-tracer-unittest.cc‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,43 @@ TEST_F(GCTracerTest, PerGenerationAllocationThroughput) {
143143
tracer->EmbedderAllocationThroughputInBytesPerMillisecond()));
144144
}
145145

146+
TEST_F(GCTracerTest, LowAllocationRateWithZeroEmbedderAllocation) {
147+
if (v8_flags.stress_incremental_marking) return;
148+
Heap* heap = i_isolate()->heap();
149+
GCTracer* tracer = heap->tracer();
150+
tracer->ResetForTesting();
151+
tracer->allocation_time_ = base::TimeTicks();
152+
153+
// Two samples 100 ms apart. Young and old generation allocate a little;
154+
// the embedder (CppHeap) allocates nothing at all, as in an embedder that
155+
// never uses cppgc (e.g. Node.js without node:vm).
156+
tracer->SampleAllocation(base::TimeTicks::FromMsTicksForTesting(0), 0, 0, 0);
157+
tracer->SampleAllocation(base::TimeTicks::FromMsTicksForTesting(100), 100,
158+
100, 0);
159+
160+
EXPECT_DOUBLE_EQ(0.0,
161+
tracer->EmbedderAllocationThroughputInBytesPerMillisecond());
162+
EXPECT_TRUE(heap->HasLowAllocationRate());
163+
}
164+
165+
TEST_F(GCTracerTest, HighAllocationRateWithEmbedderAllocation) {
166+
if (v8_flags.stress_incremental_marking) return;
167+
Heap* heap = i_isolate()->heap();
168+
GCTracer* tracer = heap->tracer();
169+
tracer->ResetForTesting();
170+
tracer->allocation_time_ = base::TimeTicks();
171+
172+
// Same as above, but the embedder allocates heavily: 1 GB over 100 ms is
173+
// ~10^7 B/ms, so the embedder term must still report a high allocation
174+
// rate (mu = 200000 / (10^7 + 200000) < 0.993).
175+
tracer->SampleAllocation(base::TimeTicks::FromMsTicksForTesting(0), 0, 0, 0);
176+
tracer->SampleAllocation(base::TimeTicks::FromMsTicksForTesting(100), 100,
177+
100, size_t{1} << 30);
178+
179+
EXPECT_GT(tracer->EmbedderAllocationThroughputInBytesPerMillisecond(), 0.0);
180+
EXPECT_FALSE(heap->HasLowAllocationRate());
181+
}
182+
146183
TEST_F(GCTracerTest, RegularScope) {
147184
if (v8_flags.stress_incremental_marking) return;
148185
GCTracer* tracer = i_isolate()->heap()->tracer();

0 commit comments

Comments
 (0)