Revert "[heap] Avoid dynamic updates of FLAG_gc_interval"

This reverts commit abcb6bb8b4.

Reason for revert: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20isolates/20029/overview

Original change's description:
> [heap] Avoid dynamic updates of FLAG_gc_interval
>
> Flags will be protected from updates after V8 initialization (in the
> future). This CL avoids any updates of the --gc-interval flag during
> runtime, and instead updates a static field on the HeapAllocator
> directly.
>
> R=​mlippautz@chromium.org
>
> Bug: v8:12887
> Change-Id: I17a495cae50a46d59a8159c6ece1558d4d61b949
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3687691
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#80998}

Bug: v8:12887
Change-Id: I18310a3f515506d617f42be7a208013957625eaf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3695559
Reviewed-by: Manos Koukoutos <manoskouk@chromium.org>
Owners-Override: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81002}
This commit is contained in:
Manos Koukoutos 2022-06-08 15:25:09 +00:00 committed by V8 LUCI CQ
parent c51a81e705
commit cc13b6c0bc
7 changed files with 11 additions and 39 deletions

View File

@ -1199,16 +1199,10 @@ DEFINE_BOOL(separate_gc_phases, false,
DEFINE_BOOL(global_gc_scheduling, true, DEFINE_BOOL(global_gc_scheduling, true,
"enable GC scheduling based on global memory") "enable GC scheduling based on global memory")
DEFINE_BOOL(gc_global, false, "always perform global GCs") DEFINE_BOOL(gc_global, false, "always perform global GCs")
// TODO(12950): The next two flags only have an effect if
// V8_ENABLE_ALLOCATION_TIMEOUT is set, so we should only define them in that
// config. That currently breaks Node's parallel/test-domain-error-types test
// though.
DEFINE_INT(random_gc_interval, 0, DEFINE_INT(random_gc_interval, 0,
"Collect garbage after random(0, X) allocations. It overrides " "Collect garbage after random(0, X) allocations. It overrides "
"gc_interval.") "gc_interval.")
DEFINE_INT(gc_interval, -1, "garbage collect after <n> allocations") DEFINE_INT(gc_interval, -1, "garbage collect after <n> allocations")
DEFINE_INT(retain_maps_for_n_gc, 2, DEFINE_INT(retain_maps_for_n_gc, 2,
"keeps maps alive for <n> old space garbage collections") "keeps maps alive for <n> old space garbage collections")
DEFINE_BOOL(trace_gc, false, DEFINE_BOOL(trace_gc, false,

View File

@ -141,18 +141,6 @@ void HeapAllocator::IncrementObjectCounters() {
#endif // DEBUG #endif // DEBUG
#ifdef V8_ENABLE_ALLOCATION_TIMEOUT #ifdef V8_ENABLE_ALLOCATION_TIMEOUT
// static
void HeapAllocator::InitializeOncePerProcess() {
SetAllocationGcInterval(FLAG_gc_interval);
}
// static
void HeapAllocator::SetAllocationGcInterval(int allocation_gc_interval) {
allocation_gc_interval_ = allocation_gc_interval;
}
// static
int HeapAllocator::allocation_gc_interval_ = -1;
void HeapAllocator::SetAllocationTimeout(int allocation_timeout) { void HeapAllocator::SetAllocationTimeout(int allocation_timeout) {
allocation_timeout_ = allocation_timeout; allocation_timeout_ = allocation_timeout;
@ -170,8 +158,8 @@ void HeapAllocator::UpdateAllocationTimeout() {
// allow the subsequent allocation attempts to go through. // allow the subsequent allocation attempts to go through.
constexpr int kFewAllocationsHeadroom = 6; constexpr int kFewAllocationsHeadroom = 6;
allocation_timeout_ = std::max(kFewAllocationsHeadroom, new_timeout); allocation_timeout_ = std::max(kFewAllocationsHeadroom, new_timeout);
} else if (allocation_gc_interval_ >= 0) { } else if (FLAG_gc_interval >= 0) {
allocation_timeout_ = allocation_gc_interval_; allocation_timeout_ = FLAG_gc_interval;
} }
} }

View File

@ -70,9 +70,6 @@ class V8_EXPORT_PRIVATE HeapAllocator final {
#ifdef V8_ENABLE_ALLOCATION_TIMEOUT #ifdef V8_ENABLE_ALLOCATION_TIMEOUT
void UpdateAllocationTimeout(); void UpdateAllocationTimeout();
void SetAllocationTimeout(int allocation_timeout); void SetAllocationTimeout(int allocation_timeout);
static void SetAllocationGcInterval(int allocation_gc_interval);
static void InitializeOncePerProcess();
#endif // V8_ENABLE_ALLOCATION_TIMEOUT #endif // V8_ENABLE_ALLOCATION_TIMEOUT
private: private:
@ -110,15 +107,10 @@ class V8_EXPORT_PRIVATE HeapAllocator final {
ConcurrentAllocator* shared_map_allocator_; ConcurrentAllocator* shared_map_allocator_;
#ifdef V8_ENABLE_ALLOCATION_TIMEOUT #ifdef V8_ENABLE_ALLOCATION_TIMEOUT
// If the {allocation_gc_interval_} is set to a positive value, this variable // If the --gc-interval flag is set to a positive value, this variable
// holds the value indicating the number of allocations remain until the // holds the value indicating the number of allocations remain until the
// next failure and garbage collection. // next failure and garbage collection.
int allocation_timeout_ = 0; int allocation_timeout_ = 0;
// The configured GC interval, initialized from --gc-interval during
// {InitializeOncePerProcess} and potentially dynamically updated by
// %SetAllocationTimeout.
static int allocation_gc_interval_;
#endif // V8_ENABLE_ALLOCATION_TIMEOUT #endif // V8_ENABLE_ALLOCATION_TIMEOUT
}; };

View File

@ -5927,9 +5927,6 @@ void Heap::InitializeHashSeed() {
// static // static
void Heap::InitializeOncePerProcess() { void Heap::InitializeOncePerProcess() {
#ifdef V8_ENABLE_ALLOCATION_TIMEOUT
HeapAllocator::InitializeOncePerProcess();
#endif
MemoryAllocator::InitializeOncePerProcess(); MemoryAllocator::InitializeOncePerProcess();
} }

View File

@ -884,12 +884,12 @@ RUNTIME_FUNCTION(Runtime_SetAllocationTimeout) {
SealHandleScope shs(isolate); SealHandleScope shs(isolate);
DCHECK(args.length() == 2 || args.length() == 3); DCHECK(args.length() == 2 || args.length() == 3);
#ifdef V8_ENABLE_ALLOCATION_TIMEOUT #ifdef V8_ENABLE_ALLOCATION_TIMEOUT
CONVERT_INT32_ARG_FUZZ_SAFE(interval, 0);
HeapAllocator::SetAllocationGcInterval(interval);
CONVERT_INT32_ARG_FUZZ_SAFE(timeout, 1); CONVERT_INT32_ARG_FUZZ_SAFE(timeout, 1);
isolate->heap()->set_allocation_timeout(timeout); isolate->heap()->set_allocation_timeout(timeout);
#endif #endif
#ifdef DEBUG #ifdef DEBUG
CONVERT_INT32_ARG_FUZZ_SAFE(interval, 0);
FLAG_gc_interval = interval;
if (args.length() == 3) { if (args.length() == 3) {
// Enable/disable inline allocation if requested. // Enable/disable inline allocation if requested.
CONVERT_BOOLEAN_ARG_FUZZ_SAFE(inline_allocation, 2); CONVERT_BOOLEAN_ARG_FUZZ_SAFE(inline_allocation, 2);

View File

@ -3023,7 +3023,7 @@ static void AddPropertyTo(int gc_count, Handle<JSObject> object,
Factory* factory = isolate->factory(); Factory* factory = isolate->factory();
Handle<String> prop_name = factory->InternalizeUtf8String(property_name); Handle<String> prop_name = factory->InternalizeUtf8String(property_name);
Handle<Smi> twenty_three(Smi::FromInt(23), isolate); Handle<Smi> twenty_three(Smi::FromInt(23), isolate);
HeapAllocator::SetAllocationGcInterval(gc_count); FLAG_gc_interval = gc_count;
FLAG_gc_global = true; FLAG_gc_global = true;
FLAG_retain_maps_for_n_gc = 0; FLAG_retain_maps_for_n_gc = 0;
CcTest::heap()->set_allocation_timeout(gc_count); CcTest::heap()->set_allocation_timeout(gc_count);
@ -4825,7 +4825,7 @@ TEST(AddInstructionChangesNewSpacePromotion) {
FLAG_allow_natives_syntax = true; FLAG_allow_natives_syntax = true;
FLAG_expose_gc = true; FLAG_expose_gc = true;
FLAG_stress_compaction = true; FLAG_stress_compaction = true;
HeapAllocator::SetAllocationGcInterval(1000); FLAG_gc_interval = 1000;
CcTest::InitializeVM(); CcTest::InitializeVM();
if (!FLAG_allocation_site_pretenuring) return; if (!FLAG_allocation_site_pretenuring) return;
v8::HandleScope scope(CcTest::isolate()); v8::HandleScope scope(CcTest::isolate());
@ -5210,6 +5210,8 @@ TEST(RetainedMapsCleanup) {
} }
TEST(PreprocessStackTrace) { TEST(PreprocessStackTrace) {
// Do not automatically trigger early GC.
FLAG_gc_interval = -1;
CcTest::InitializeVM(); CcTest::InitializeVM();
v8::HandleScope scope(CcTest::isolate()); v8::HandleScope scope(CcTest::isolate());
v8::TryCatch try_catch(CcTest::isolate()); v8::TryCatch try_catch(CcTest::isolate());
@ -5239,6 +5241,7 @@ TEST(PreprocessStackTrace) {
} }
} }
void AllocateInSpace(Isolate* isolate, size_t bytes, AllocationSpace space) { void AllocateInSpace(Isolate* isolate, size_t bytes, AllocationSpace space) {
CHECK_LE(FixedArray::kHeaderSize, bytes); CHECK_LE(FixedArray::kHeaderSize, bytes);
CHECK(IsAligned(bytes, kTaggedSize)); CHECK(IsAligned(bytes, kTaggedSize));

View File

@ -24152,11 +24152,9 @@ TEST(CreateSyntheticModule) {
} }
TEST(CreateSyntheticModuleGC) { TEST(CreateSyntheticModuleGC) {
#ifdef V8_ENABLE_ALLOCATION_TIMEOUT
// Try to make sure that CreateSyntheticModule() deals well with a GC // Try to make sure that CreateSyntheticModule() deals well with a GC
// happening during its execution. // happening during its execution.
i::HeapAllocator::SetAllocationGcInterval(10); i::FLAG_gc_interval = 10;
#endif
i::FLAG_inline_new = false; i::FLAG_inline_new = false;
LocalContext env; LocalContext env;