diff --git a/src/api/api.cc b/src/api/api.cc index 66c9712966..cd9492274f 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -8481,7 +8481,7 @@ void Isolate::Initialize(Isolate* isolate, } if (params.experimental_attach_to_shared_isolate != nullptr) { - i_isolate->AttachToSharedIsolate(reinterpret_cast( + i_isolate->set_shared_isolate(reinterpret_cast( params.experimental_attach_to_shared_isolate)); } diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index 22942c7470..828ffcca13 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -3125,6 +3125,7 @@ bool Isolate::LogObjectRelocation() { void Isolate::Deinit() { TRACE_ISOLATE(deinit); + DisallowHeapAllocation no_allocation; tracing_cpu_profiler_.reset(); if (FLAG_stress_sampling_allocation_profiler > 0) { @@ -3210,9 +3211,7 @@ void Isolate::Deinit() { main_thread_local_isolate_->heap()->FreeLinearAllocationArea(); - if (shared_isolate_) { - DetachFromSharedIsolate(); - } + DetachFromSharedIsolate(); heap_.TearDown(); @@ -3659,12 +3658,6 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data, date_cache_ = new DateCache(); heap_profiler_ = new HeapProfiler(heap()); interpreter_ = new interpreter::Interpreter(this); - if (OwnsStringTable()) { - string_table_ = std::make_shared(this); - } else { - DCHECK_NOT_NULL(shared_isolate_); - string_table_ = shared_isolate_->string_table_; - } bigint_processor_ = bigint::Processor::New(new BigIntPlatform(this)); compiler_dispatcher_ = new LazyCompileDispatcher( @@ -3684,12 +3677,33 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data, stack_guard()->InitThread(lock); } + // Create LocalIsolate/LocalHeap for the main thread and set state to Running. + main_thread_local_isolate_.reset(new LocalIsolate(this, ThreadKind::kMain)); + main_thread_local_heap()->Unpark(); + + // The main thread LocalHeap needs to be set up when attaching to the shared + // isolate. Otherwise a global safepoint would find an isolate without + // LocalHeaps and not wait until this thread is ready for a GC. + AttachToSharedIsolate(); + + // We need to ensure that we do not let a shared GC run before this isolate is + // fully set up. + DisallowSafepoints no_shared_gc; + // SetUp the object heap. DCHECK(!heap_.HasBeenSetUp()); - heap_.SetUp(); + heap_.SetUp(main_thread_local_heap()); ReadOnlyHeap::SetUp(this, read_only_snapshot_data, can_rehash); heap_.SetUpSpaces(); + if (OwnsStringTable()) { + string_table_ = std::make_shared(this); + } else { + // Only refer to shared string table after attaching to the shared isolate. + DCHECK_NOT_NULL(shared_isolate()); + string_table_ = shared_isolate()->string_table_; + } + if (V8_SHORT_BUILTIN_CALLS_BOOL && FLAG_short_builtin_calls) { // Check if the system has more than 4GB of physical memory by comparing the // old space size with respective threshold value. @@ -3714,14 +3728,6 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data, } } - // Create LocalIsolate/LocalHeap for the main thread and set state to Running. - main_thread_local_isolate_.reset(new LocalIsolate(this, ThreadKind::kMain)); - main_thread_local_heap()->Unpark(); - - heap_.InitializeMainThreadLocalHeap(main_thread_local_heap()); - - if (shared_isolate_) heap()->InitSharedSpaces(); - isolate_data_.external_reference_table()->Init(this); #if V8_ENABLE_WEBASSEMBLY @@ -5163,19 +5169,30 @@ Address Isolate::store_to_stack_count_address(const char* function_name) { return reinterpret_cast
(&map[name].second); } -void Isolate::AttachToSharedIsolate(Isolate* shared) { - DCHECK(shared->is_shared()); - DCHECK_NULL(shared_isolate_); - DCHECK(!heap_.HasBeenSetUp()); - shared->AppendAsClientIsolate(this); - shared_isolate_ = shared; +void Isolate::AttachToSharedIsolate() { + DCHECK(!attached_to_shared_isolate_); + + if (shared_isolate_) { + DCHECK(shared_isolate_->is_shared()); + shared_isolate_->AppendAsClientIsolate(this); + } + +#if DEBUG + attached_to_shared_isolate_ = true; +#endif // DEBUG } void Isolate::DetachFromSharedIsolate() { - DCHECK_NOT_NULL(shared_isolate_); - shared_isolate_->RemoveAsClientIsolate(this); - shared_isolate_ = nullptr; - heap()->DeinitSharedSpaces(); + DCHECK(attached_to_shared_isolate_); + + if (shared_isolate_) { + shared_isolate_->RemoveAsClientIsolate(this); + shared_isolate_ = nullptr; + } + +#if DEBUG + attached_to_shared_isolate_ = false; +#endif // DEBUG } void Isolate::AppendAsClientIsolate(Isolate* client) { diff --git a/src/execution/isolate.h b/src/execution/isolate.h index 5aaf5084cc..b286362cc5 100644 --- a/src/execution/isolate.h +++ b/src/execution/isolate.h @@ -1827,11 +1827,18 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { using IsDebugActive = HasAsyncEventDelegate::Next; }; - bool is_shared() { return is_shared_; } - Isolate* shared_isolate() { return shared_isolate_; } + bool is_shared() const { return is_shared_; } + Isolate* shared_isolate() const { + DCHECK(attached_to_shared_isolate_); + return shared_isolate_; + } - void AttachToSharedIsolate(Isolate* shared); - void DetachFromSharedIsolate(); + void set_shared_isolate(Isolate* shared_isolate) { + DCHECK(shared_isolate->is_shared()); + DCHECK_NULL(shared_isolate_); + DCHECK(!attached_to_shared_isolate_); + shared_isolate_ = shared_isolate; + } bool HasClientIsolates() const { return client_isolate_head_; } @@ -1967,6 +1974,12 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { // Returns the Exception sentinel. Object ThrowInternal(Object exception, MessageLocation* location); + // These methods add/remove the isolate to/from the list of clients in the + // shared isolate. Isolates in the client list need to participate in a global + // safepoint. + void AttachToSharedIsolate(); + void DetachFromSharedIsolate(); + // Methods for appending and removing to/from client isolates list. void AppendAsClientIsolate(Isolate* client); void RemoveAsClientIsolate(Isolate* client); @@ -2262,6 +2275,13 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { // isolates or when no shared isolate is used. Isolate* shared_isolate_ = nullptr; +#if DEBUG + // Set to true once during isolate initialization right when attaching to the + // shared isolate. If there was no shared isolate given it will still be set + // to true. After this point invocations of shared_isolate() are valid. + bool attached_to_shared_isolate_ = false; +#endif // DEBUG + // A shared isolate will use these two fields to track all its client // isolates. base::Mutex client_isolate_mutex_; diff --git a/src/heap/heap.cc b/src/heap/heap.cc index d9f227e4de..f5353f0035 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -2240,6 +2240,7 @@ size_t Heap::PerformGarbageCollection( } void Heap::CollectSharedGarbage(GarbageCollectionReason gc_reason) { + CHECK(deserialization_complete()); DCHECK(!IsShared()); DCHECK_NOT_NULL(isolate()->shared_isolate()); @@ -2266,6 +2267,7 @@ void Heap::PerformSharedGarbageCollection(Isolate* initiator, : IsolateSafepoint::StopMainThread::kYes; client_heap->safepoint()->EnterSafepointScope(stop_main_thread); + DCHECK(client_heap->deserialization_complete()); client_heap->shared_old_allocator_->FreeLinearAllocationArea(); client_heap->shared_map_allocator_->FreeLinearAllocationArea(); @@ -5512,7 +5514,10 @@ HeapObject Heap::AllocateRawWithRetryOrFailSlowPath( FatalProcessOutOfMemory("CALL_AND_RETRY_LAST"); } -void Heap::SetUp() { +void Heap::SetUp(LocalHeap* main_thread_local_heap) { + DCHECK_NULL(main_thread_local_heap_); + main_thread_local_heap_ = main_thread_local_heap; + #ifdef V8_ENABLE_ALLOCATION_TIMEOUT allocation_timeout_ = NextAllocationTimeout(); #endif @@ -5705,11 +5710,18 @@ void Heap::SetUpSpaces() { } write_protect_code_memory_ = FLAG_write_protect_code_memory; -} -void Heap::InitializeMainThreadLocalHeap(LocalHeap* main_thread_local_heap) { - DCHECK_NULL(main_thread_local_heap_); - main_thread_local_heap_ = main_thread_local_heap; + if (isolate()->shared_isolate()) { + shared_old_space_ = isolate()->shared_isolate()->heap()->old_space(); + shared_old_allocator_.reset( + new ConcurrentAllocator(main_thread_local_heap(), shared_old_space_)); + + shared_map_space_ = isolate()->shared_isolate()->heap()->map_space(); + shared_map_allocator_.reset( + new ConcurrentAllocator(main_thread_local_heap(), shared_map_space_)); + } + + main_thread_local_heap()->SetUpMainThread(); } void Heap::InitializeHashSeed() { @@ -5992,6 +6004,12 @@ void Heap::TearDown() { allocation_sites_to_pretenure_.reset(); + shared_old_space_ = nullptr; + shared_old_allocator_.reset(); + + shared_map_space_ = nullptr; + shared_map_allocator_.reset(); + for (int i = FIRST_MUTABLE_SPACE; i <= LAST_MUTABLE_SPACE; i++) { delete space_[i]; space_[i] = nullptr; @@ -6013,24 +6031,6 @@ void Heap::TearDown() { memory_allocator_.reset(); } -void Heap::InitSharedSpaces() { - shared_old_space_ = isolate()->shared_isolate()->heap()->old_space(); - shared_old_allocator_.reset( - new ConcurrentAllocator(main_thread_local_heap(), shared_old_space_)); - - shared_map_space_ = isolate()->shared_isolate()->heap()->map_space(); - shared_map_allocator_.reset( - new ConcurrentAllocator(main_thread_local_heap(), shared_map_space_)); -} - -void Heap::DeinitSharedSpaces() { - shared_old_space_ = nullptr; - shared_old_allocator_.reset(); - - shared_map_space_ = nullptr; - shared_map_allocator_.reset(); -} - void Heap::AddGCPrologueCallback(v8::Isolate::GCCallbackWithData callback, GCType gc_type, void* data) { DCHECK_NOT_NULL(callback); diff --git a/src/heap/heap.h b/src/heap/heap.h index 808b00dc03..a5e3630b8c 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -840,7 +840,7 @@ class Heap { void ConfigureHeapDefault(); // Prepares the heap, setting up for deserialization. - void SetUp(); + void SetUp(LocalHeap* main_thread_local_heap); // Sets read-only heap and space. void SetUpFromReadOnlyHeap(ReadOnlyHeap* ro_heap); @@ -872,12 +872,6 @@ class Heap { // Returns whether SetUp has been called. bool HasBeenSetUp() const; - // Initialializes shared spaces. - void InitSharedSpaces(); - - // Removes shared spaces again. - void DeinitSharedSpaces(); - // =========================================================================== // Getters for spaces. ======================================================= // =========================================================================== diff --git a/src/heap/local-heap.cc b/src/heap/local-heap.cc index d6aa5d4c05..e5edc993c9 100644 --- a/src/heap/local-heap.cc +++ b/src/heap/local-heap.cc @@ -13,6 +13,7 @@ #include "src/execution/isolate.h" #include "src/handles/local-handles.h" #include "src/heap/collection-barrier.h" +#include "src/heap/concurrent-allocator.h" #include "src/heap/gc-tracer.h" #include "src/heap/heap-inl.h" #include "src/heap/heap-write-barrier.h" @@ -51,10 +52,8 @@ LocalHeap::LocalHeap(Heap* heap, ThreadKind kind, prev_(nullptr), next_(nullptr), handles_(new LocalHandles), - persistent_handles_(std::move(persistent_handles)), - marking_barrier_(new MarkingBarrier(this)), - old_space_allocator_(this, heap->old_space()), - code_space_allocator_(this, heap->code_space()) { + persistent_handles_(std::move(persistent_handles)) { + if (!is_main_thread()) SetUp(); heap_->safepoint()->AddLocalHeap(this, [this] { if (!is_main_thread()) { WriteBarrier::SetForThread(marking_barrier_.get()); @@ -77,8 +76,7 @@ LocalHeap::~LocalHeap() { EnsureParkedBeforeDestruction(); heap_->safepoint()->RemoveLocalHeap(this, [this] { - old_space_allocator_.FreeLinearAllocationArea(); - code_space_allocator_.FreeLinearAllocationArea(); + FreeLinearAllocationArea(); if (!is_main_thread()) { marking_barrier_->Publish(); @@ -94,6 +92,26 @@ LocalHeap::~LocalHeap() { DCHECK(gc_epilogue_callbacks_.empty()); } +void LocalHeap::SetUpMainThreadForTesting() { SetUpMainThread(); } + +void LocalHeap::SetUpMainThread() { + DCHECK(is_main_thread()); + SetUp(); +} + +void LocalHeap::SetUp() { + DCHECK_NULL(old_space_allocator_); + old_space_allocator_ = + std::make_unique(this, heap_->old_space()); + + DCHECK_NULL(code_space_allocator_); + code_space_allocator_ = + std::make_unique(this, heap_->code_space()); + + DCHECK_NULL(marking_barrier_); + marking_barrier_ = std::make_unique(this); +} + void LocalHeap::EnsurePersistentHandles() { if (!persistent_handles_) { persistent_handles_.reset( @@ -227,23 +245,23 @@ void LocalHeap::SafepointSlowPath() { } void LocalHeap::FreeLinearAllocationArea() { - old_space_allocator_.FreeLinearAllocationArea(); - code_space_allocator_.FreeLinearAllocationArea(); + old_space_allocator_->FreeLinearAllocationArea(); + code_space_allocator_->FreeLinearAllocationArea(); } void LocalHeap::MakeLinearAllocationAreaIterable() { - old_space_allocator_.MakeLinearAllocationAreaIterable(); - code_space_allocator_.MakeLinearAllocationAreaIterable(); + old_space_allocator_->MakeLinearAllocationAreaIterable(); + code_space_allocator_->MakeLinearAllocationAreaIterable(); } void LocalHeap::MarkLinearAllocationAreaBlack() { - old_space_allocator_.MarkLinearAllocationAreaBlack(); - code_space_allocator_.MarkLinearAllocationAreaBlack(); + old_space_allocator_->MarkLinearAllocationAreaBlack(); + code_space_allocator_->MarkLinearAllocationAreaBlack(); } void LocalHeap::UnmarkLinearAllocationArea() { - old_space_allocator_.UnmarkLinearAllocationArea(); - code_space_allocator_.UnmarkLinearAllocationArea(); + old_space_allocator_->UnmarkLinearAllocationArea(); + code_space_allocator_->UnmarkLinearAllocationArea(); } bool LocalHeap::TryPerformCollection() { diff --git a/src/heap/local-heap.h b/src/heap/local-heap.h index 5a00101161..87dd566939 100644 --- a/src/heap/local-heap.h +++ b/src/heap/local-heap.h @@ -93,8 +93,12 @@ class V8_EXPORT_PRIVATE LocalHeap { Heap* AsHeap() { return heap(); } MarkingBarrier* marking_barrier() { return marking_barrier_.get(); } - ConcurrentAllocator* old_space_allocator() { return &old_space_allocator_; } - ConcurrentAllocator* code_space_allocator() { return &code_space_allocator_; } + ConcurrentAllocator* old_space_allocator() { + return old_space_allocator_.get(); + } + ConcurrentAllocator* code_space_allocator() { + return code_space_allocator_.get(); + } // Mark/Unmark linear allocation areas black. Used for black allocation. void MarkLinearAllocationAreaBlack(); @@ -150,6 +154,9 @@ class V8_EXPORT_PRIVATE LocalHeap { void AddGCEpilogueCallback(GCEpilogueCallback* callback, void* data); void RemoveGCEpilogueCallback(GCEpilogueCallback* callback, void* data); + // Used to make SetupMainThread() available to unit tests. + void SetUpMainThreadForTesting(); + private: using ParkedBit = base::BitField8; using SafepointRequestedBit = ParkedBit::Next; @@ -248,7 +255,7 @@ class V8_EXPORT_PRIVATE LocalHeap { AllocationAlignment alignment); void Park() { - DCHECK(AllowGarbageCollection::IsAllowed()); + DCHECK(AllowSafepoints::IsAllowed()); ThreadState expected = ThreadState::Running(); if (!state_.CompareExchangeWeak(expected, ThreadState::Parked())) { ParkSlowPath(); @@ -256,7 +263,7 @@ class V8_EXPORT_PRIVATE LocalHeap { } void Unpark() { - DCHECK(AllowGarbageCollection::IsAllowed()); + DCHECK(AllowSafepoints::IsAllowed()); ThreadState expected = ThreadState::Parked(); if (!state_.CompareExchangeWeak(expected, ThreadState::Running())) { UnparkSlowPath(); @@ -272,6 +279,9 @@ class V8_EXPORT_PRIVATE LocalHeap { void InvokeGCEpilogueCallbacksInSafepoint(); + void SetUpMainThread(); + void SetUp(); + Heap* heap_; bool is_main_thread_; @@ -289,8 +299,8 @@ class V8_EXPORT_PRIVATE LocalHeap { std::vector> gc_epilogue_callbacks_; - ConcurrentAllocator old_space_allocator_; - ConcurrentAllocator code_space_allocator_; + std::unique_ptr old_space_allocator_; + std::unique_ptr code_space_allocator_; friend class CollectionBarrier; friend class ConcurrentAllocator; diff --git a/src/heap/safepoint.cc b/src/heap/safepoint.cc index 428b37df69..bf3e5eaf95 100644 --- a/src/heap/safepoint.cc +++ b/src/heap/safepoint.cc @@ -39,6 +39,9 @@ void IsolateSafepoint::EnterSafepointScope(StopMainThread stop_main_thread) { int running = 0; + // There needs to be at least one LocalHeap for the main thread. + DCHECK_NOT_NULL(local_heaps_head_); + for (LocalHeap* local_heap = local_heaps_head_; local_heap; local_heap = local_heap->next_) { if (local_heap->is_main_thread() && diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc index b152990abe..5dbea8b197 100644 --- a/test/cctest/test-serialize.cc +++ b/test/cctest/test-serialize.cc @@ -159,15 +159,14 @@ class TestSerializer { SnapshotData shared_space_snapshot(blobs.shared_space); const bool kEnableSerializer = false; const bool kGenerateHeap = false; - if (is_shared) CHECK_NULL(shared_isolate); + CHECK_IMPLIES(is_shared, !shared_isolate); v8::Isolate* v8_isolate = NewIsolate(kEnableSerializer, kGenerateHeap, is_shared); v8::Isolate::Scope isolate_scope(v8_isolate); i::Isolate* isolate = reinterpret_cast(v8_isolate); if (shared_isolate) { CHECK(!is_shared); - isolate->AttachToSharedIsolate( - reinterpret_cast(shared_isolate)); + isolate->set_shared_isolate(reinterpret_cast(shared_isolate)); } isolate->Init(&startup_snapshot, &read_only_snapshot, &shared_space_snapshot, false); diff --git a/test/unittests/heap/local-heap-unittest.cc b/test/unittests/heap/local-heap-unittest.cc index a009d9433c..1414c7b0b8 100644 --- a/test/unittests/heap/local-heap-unittest.cc +++ b/test/unittests/heap/local-heap-unittest.cc @@ -29,6 +29,7 @@ TEST_F(LocalHeapTest, Current) { { LocalHeap lh(heap, ThreadKind::kMain); + lh.SetUpMainThreadForTesting(); CHECK_NULL(LocalHeap::Current()); } @@ -36,6 +37,7 @@ TEST_F(LocalHeapTest, Current) { { LocalHeap lh(heap, ThreadKind::kMain); + lh.SetUpMainThreadForTesting(); CHECK_NULL(LocalHeap::Current()); } @@ -67,6 +69,7 @@ TEST_F(LocalHeapTest, CurrentBackground) { CHECK_NULL(LocalHeap::Current()); { LocalHeap lh(heap, ThreadKind::kMain); + lh.SetUpMainThreadForTesting(); auto thread = std::make_unique(heap); CHECK(thread->Start()); CHECK_NULL(LocalHeap::Current()); @@ -157,6 +160,7 @@ class BackgroundThreadForGCEpilogue final : public v8::base::Thread { TEST_F(LocalHeapTest, GCEpilogue) { Heap* heap = i_isolate()->heap(); LocalHeap lh(heap, ThreadKind::kMain); + lh.SetUpMainThreadForTesting(); std::array epilogue; { UnparkedScope unparked(&lh);