diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index ff7c8b07f4..0989f4fa5d 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -56,9 +56,6 @@ #include "src/handles/global-handles-inl.h" #include "src/handles/persistent-handles.h" #include "src/heap/heap-inl.h" -#include "src/heap/heap.h" -#include "src/heap/local-heap.h" -#include "src/heap/parked-scope.h" #include "src/heap/read-only-heap.h" #include "src/heap/safepoint.h" #include "src/ic/stub-cache.h" @@ -3209,18 +3206,7 @@ void Isolate::Deinit() { // This stops cancelable tasks (i.e. concurrent marking tasks). // Stop concurrent tasks before destroying resources since they might still // use those. - { - IgnoreLocalGCRequests ignore_gc_requests(heap()); - ParkedScope parked_scope(main_thread_local_heap()); - cancelable_task_manager()->CancelAndWait(); - } - - { - // This isolate might have to park for a shared GC initiated by another - // client isolate before it can actually detach from the shared isolate. - AllowGarbageCollection allow_shared_gc; - DetachFromSharedIsolate(); - } + cancelable_task_manager()->CancelAndWait(); ReleaseSharedPtrs(); @@ -3248,6 +3234,10 @@ void Isolate::Deinit() { // updated anymore. DumpAndResetStats(); + main_thread_local_isolate_->heap()->FreeLinearAllocationArea(); + + DetachFromSharedIsolate(); + heap_.TearDown(); main_thread_local_isolate_.reset(); @@ -3728,11 +3718,7 @@ 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)); - - { - IgnoreLocalGCRequests ignore_gc_requests(heap()); - main_thread_local_heap()->Unpark(); - } + 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 diff --git a/src/heap/collection-barrier.cc b/src/heap/collection-barrier.cc index 3cf8f41c43..3a1a1e5947 100644 --- a/src/heap/collection-barrier.cc +++ b/src/heap/collection-barrier.cc @@ -22,17 +22,14 @@ bool CollectionBarrier::WasGCRequested() { return collection_requested_.load(); } -bool CollectionBarrier::TryRequestGC() { +void CollectionBarrier::RequestGC() { base::MutexGuard guard(&mutex_); - if (shutdown_requested_) return false; bool was_already_requested = collection_requested_.exchange(true); if (!was_already_requested) { CHECK(!timer_.IsStarted()); timer_.Start(); } - - return true; } class BackgroundCollectionInterruptTask : public CancelableTask { @@ -62,19 +59,8 @@ void CollectionBarrier::NotifyShutdownRequested() { void CollectionBarrier::ResumeThreadsAwaitingCollection() { base::MutexGuard guard(&mutex_); - DCHECK(!timer_.IsStarted()); collection_requested_.store(false); block_for_collection_ = false; - collection_performed_ = true; - cv_wakeup_.NotifyAll(); -} - -void CollectionBarrier::CancelCollectionAndResumeThreads() { - base::MutexGuard guard(&mutex_); - if (timer_.IsStarted()) timer_.Stop(); - collection_requested_.store(false); - block_for_collection_ = false; - collection_performed_ = false; cv_wakeup_.NotifyAll(); } @@ -86,10 +72,6 @@ bool CollectionBarrier::AwaitCollectionBackground(LocalHeap* local_heap) { // set before the next GC. base::MutexGuard guard(&mutex_); if (shutdown_requested_) return false; - - // Collection was cancelled by the main thread. - if (!collection_requested_.load()) return false; - first_thread = !block_for_collection_; block_for_collection_ = true; CHECK(timer_.IsStarted()); @@ -106,8 +88,7 @@ bool CollectionBarrier::AwaitCollectionBackground(LocalHeap* local_heap) { cv_wakeup_.Wait(&mutex_); } - // Collection may have been cancelled while blocking for it. - return collection_performed_; + return true; } void CollectionBarrier::ActivateStackGuardAndPostTask() { diff --git a/src/heap/collection-barrier.h b/src/heap/collection-barrier.h index fd894324a6..ee7fd33ad1 100644 --- a/src/heap/collection-barrier.h +++ b/src/heap/collection-barrier.h @@ -27,10 +27,8 @@ class CollectionBarrier { // Returns true when collection was requested. bool WasGCRequested(); - // Requests a GC from the main thread. Returns whether GC was successfully - // requested. Requesting a GC can fail when isolate shutdown was already - // initiated. - bool TryRequestGC(); + // Requests a GC from the main thread. + void RequestGC(); // Resumes all threads waiting for GC when tear down starts. void NotifyShutdownRequested(); @@ -41,11 +39,7 @@ class CollectionBarrier { // Resumes threads waiting for collection. void ResumeThreadsAwaitingCollection(); - // Cancels collection if one was requested and resumes threads waiting for GC. - void CancelCollectionAndResumeThreads(); - // This is the method use by background threads to request and wait for GC. - // Returns whether a GC was performed. bool AwaitCollectionBackground(LocalHeap* local_heap); private: @@ -56,21 +50,8 @@ class CollectionBarrier { base::Mutex mutex_; base::ConditionVariable cv_wakeup_; base::ElapsedTimer timer_; - - // Flag that main thread checks whether a GC was requested from the background - // thread. std::atomic collection_requested_{false}; - - // This flag is used to detect whether to block for the GC. Only set if the - // main thread was actually running and is unset when GC resumes background - // threads. bool block_for_collection_ = false; - - // Set to true when a GC was performed, false in case it was canceled because - // the main thread parked itself without running the GC. - bool collection_performed_ = false; - - // Will be set as soon as Isolate starts tear down. bool shutdown_requested_ = false; }; diff --git a/src/heap/concurrent-allocator.cc b/src/heap/concurrent-allocator.cc index 2d0be32599..546a1cf719 100644 --- a/src/heap/concurrent-allocator.cc +++ b/src/heap/concurrent-allocator.cc @@ -124,7 +124,7 @@ bool ConcurrentAllocator::EnsureLab(AllocationOrigin origin) { local_heap_, kLabSize, kMaxLabSize, kWordAligned, origin); if (!result) return false; - if (IsBlackAllocationEnabled()) { + if (local_heap_->heap()->incremental_marking()->black_allocation()) { Address top = result->first; Address limit = top + result->second; Page::FromAllocationAreaAddress(top)->CreateBlackAreaBackground(top, limit); @@ -149,19 +149,13 @@ AllocationResult ConcurrentAllocator::AllocateOutsideLab( HeapObject object = HeapObject::FromAddress(result->first); - if (IsBlackAllocationEnabled()) { - owning_heap()->incremental_marking()->MarkBlackBackground(object, - object_size); + if (local_heap_->heap()->incremental_marking()->black_allocation()) { + local_heap_->heap()->incremental_marking()->MarkBlackBackground( + object, object_size); } return AllocationResult(object); } -bool ConcurrentAllocator::IsBlackAllocationEnabled() const { - return owning_heap()->incremental_marking()->black_allocation(); -} - -Heap* ConcurrentAllocator::owning_heap() const { return space_->heap(); } - } // namespace internal } // namespace v8 diff --git a/src/heap/concurrent-allocator.h b/src/heap/concurrent-allocator.h index bf596cf6de..fe6144eb7e 100644 --- a/src/heap/concurrent-allocator.h +++ b/src/heap/concurrent-allocator.h @@ -63,12 +63,6 @@ class ConcurrentAllocator { V8_EXPORT_PRIVATE AllocationResult AllocateOutsideLab( int object_size, AllocationAlignment alignment, AllocationOrigin origin); - bool IsBlackAllocationEnabled() const; - - // Returns the Heap of space_. This might differ from the LocalHeap's Heap for - // shared spaces. - Heap* owning_heap() const; - LocalHeap* const local_heap_; PagedSpace* const space_; LocalAllocationBuffer lab_; diff --git a/src/heap/gc-tracer.cc b/src/heap/gc-tracer.cc index 655930859a..28057413e8 100644 --- a/src/heap/gc-tracer.cc +++ b/src/heap/gc-tracer.cc @@ -563,12 +563,11 @@ void GCTracer::Print() const { Output( "[%d:%p] " "%8.0f ms: " - "%s%s%s %.1f (%.1f) -> %.1f (%.1f) MB, " + "%s%s %.1f (%.1f) -> %.1f (%.1f) MB, " "%.1f / %.1f ms %s (average mu = %.3f, current mu = %.3f) %s %s\n", base::OS::GetCurrentProcessId(), reinterpret_cast(heap_->isolate()), - heap_->isolate()->time_millis_since_init(), - heap_->IsShared() ? "Shared " : "", current_.TypeName(false), + heap_->isolate()->time_millis_since_init(), current_.TypeName(false), current_.reduce_memory ? " (reduce)" : "", static_cast(current_.start_object_size) / MB, static_cast(current_.start_memory_size) / MB, diff --git a/src/heap/heap-inl.h b/src/heap/heap-inl.h index da18f587c4..ec3da82d61 100644 --- a/src/heap/heap-inl.h +++ b/src/heap/heap-inl.h @@ -867,16 +867,6 @@ CodePageMemoryModificationScope::~CodePageMemoryModificationScope() { } } -IgnoreLocalGCRequests::IgnoreLocalGCRequests(Heap* heap) : heap_(heap) { - DCHECK_EQ(ThreadId::Current(), heap_->isolate()->thread_id()); - heap_->ignore_local_gc_requests_depth_++; -} - -IgnoreLocalGCRequests::~IgnoreLocalGCRequests() { - DCHECK_GT(heap_->ignore_local_gc_requests_depth_, 0); - heap_->ignore_local_gc_requests_depth_--; -} - } // namespace internal } // namespace v8 diff --git a/src/heap/heap.cc b/src/heap/heap.cc index fea0afb9f4..a124082c97 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -67,7 +67,6 @@ #include "src/heap/objects-visiting-inl.h" #include "src/heap/objects-visiting.h" #include "src/heap/paged-spaces-inl.h" -#include "src/heap/parked-scope.h" #include "src/heap/read-only-heap.h" #include "src/heap/remembered-set.h" #include "src/heap/safepoint.h" @@ -1924,7 +1923,6 @@ void Heap::StartIncrementalMarking(int gc_flags, CppHeap::From(cpp_heap())->FinishSweepingIfRunning(); } - IgnoreLocalGCRequests ignore_gc_requests(this); SafepointScope safepoint(this); #ifdef DEBUG @@ -2158,13 +2156,7 @@ size_t Heap::PerformGarbageCollection( TRACE_GC_EPOCH(tracer(), CollectorScopeId(collector), ThreadKind::kMain); - base::Optional safepoint_scope; - - { - AllowGarbageCollection allow_shared_gc; - IgnoreLocalGCRequests ignore_gc_requests(this); - safepoint_scope.emplace(this); - } + SafepointScope safepoint_scope(this); collection_barrier_->StopTimeToCollectionTimer(); @@ -5482,16 +5474,14 @@ HeapObject Heap::AllocateRawWithRetryOrFailSlowPath( isolate()->counters()->gc_last_resort_from_handles()->Increment(); if (IsSharedAllocationType(allocation)) { CollectSharedGarbage(GarbageCollectionReason::kLastResort); - - AlwaysAllocateScope scope(isolate()->shared_isolate()->heap()); - alloc = AllocateRaw(size, allocation, origin, alignment); } else { CollectAllAvailableGarbage(GarbageCollectionReason::kLastResort); + } + { AlwaysAllocateScope scope(this); alloc = AllocateRaw(size, allocation, origin, alignment); } - if (alloc.To(&result)) { DCHECK(result != ReadOnlyRoots(this).exception()); return result; @@ -5885,21 +5875,11 @@ void Heap::StartTearDown() { // threads finish. collection_barrier_->NotifyShutdownRequested(); - // Main thread isn't going to allocate anymore. - main_thread_local_heap()->FreeLinearAllocationArea(); - - if (isolate()->shared_isolate()) { - // Free LABs before detaching from the shared isolate. - shared_old_allocator_->FreeLinearAllocationArea(); - shared_map_allocator_->FreeLinearAllocationArea(); - } - #ifdef VERIFY_HEAP // {StartTearDown} is called fairly early during Isolate teardown, so it's // a good time to run heap verification (if requested), before starting to // tear down parts of the Isolate. if (FLAG_verify_heap) { - IgnoreLocalGCRequests ignore_gc_requests(this); SafepointScope scope(this); Verify(); } diff --git a/src/heap/heap.h b/src/heap/heap.h index 04c068480c..3bc97c050d 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -687,10 +687,6 @@ class Heap { bool IsTearingDown() const { return gc_state() == TEAR_DOWN; } bool force_oom() const { return force_oom_; } - bool ignore_local_gc_requests() const { - return ignore_local_gc_requests_depth_ > 0; - } - inline bool IsInGCPostProcessing() { return gc_post_processing_depth_ > 0; } bool IsGCWithoutStack() const; @@ -2450,8 +2446,6 @@ class Heap { std::unique_ptr collection_barrier_; - int ignore_local_gc_requests_depth_ = 0; - int gc_callbacks_depth_ = 0; bool deserialization_complete_ = false; @@ -2508,7 +2502,6 @@ class Heap { friend class GCTracer; friend class HeapObjectIterator; friend class ScavengeTaskObserver; - friend class IgnoreLocalGCRequests; friend class IncrementalMarking; friend class IncrementalMarkingJob; friend class LargeObjectSpace; @@ -2667,15 +2660,6 @@ class V8_NODISCARD CodePageMemoryModificationScope { DISALLOW_GARBAGE_COLLECTION(no_heap_allocation_) }; -class V8_NODISCARD IgnoreLocalGCRequests { - public: - explicit inline IgnoreLocalGCRequests(Heap* heap); - inline ~IgnoreLocalGCRequests(); - - private: - Heap* heap_; -}; - // Visitor class to verify interior pointers in spaces that do not contain // or care about intergenerational references. All heap object pointers have to // point into the heap to a location that has a map pointer at its first word. diff --git a/src/heap/local-heap.cc b/src/heap/local-heap.cc index 6b77267b5f..e5edc993c9 100644 --- a/src/heap/local-heap.cc +++ b/src/heap/local-heap.cc @@ -8,7 +8,6 @@ #include #include "src/base/logging.h" -#include "src/base/optional.h" #include "src/base/platform/mutex.h" #include "src/common/globals.h" #include "src/execution/isolate.h" @@ -18,7 +17,6 @@ #include "src/heap/gc-tracer.h" #include "src/heap/heap-inl.h" #include "src/heap/heap-write-barrier.h" -#include "src/heap/heap.h" #include "src/heap/local-heap-inl.h" #include "src/heap/marking-barrier.h" #include "src/heap/parked-scope.h" @@ -175,42 +173,13 @@ void LocalHeap::ParkSlowPath() { DCHECK(current_state.IsRunning()); if (is_main_thread()) { - DCHECK(current_state.IsSafepointRequested() || - current_state.IsCollectionRequested()); - - if (current_state.IsSafepointRequested()) { - ThreadState old_state = state_.SetParked(); - heap_->safepoint()->NotifyPark(); - if (old_state.IsCollectionRequested()) - heap_->collection_barrier_->CancelCollectionAndResumeThreads(); - return; - } - - if (current_state.IsCollectionRequested()) { - if (!heap()->ignore_local_gc_requests()) { - heap_->CollectGarbageForBackground(this); - continue; - } - - DCHECK(!current_state.IsSafepointRequested()); - - if (state_.CompareExchangeStrong(current_state, - current_state.SetParked())) { - heap_->collection_barrier_->CancelCollectionAndResumeThreads(); - return; - } else { - continue; - } - } + DCHECK(current_state.IsCollectionRequested()); + heap_->CollectGarbageForBackground(this); } else { DCHECK(current_state.IsSafepointRequested()); DCHECK(!current_state.IsCollectionRequested()); - - ThreadState old_state = state_.SetParked(); - CHECK(old_state.IsRunning()); - CHECK(old_state.IsSafepointRequested()); - CHECK(!old_state.IsCollectionRequested()); - + CHECK(state_.CompareExchangeStrong(current_state, + current_state.SetParked())); heap_->safepoint()->NotifyPark(); return; } @@ -227,107 +196,54 @@ void LocalHeap::UnparkSlowPath() { DCHECK(current_state.IsParked()); if (is_main_thread()) { - DCHECK(current_state.IsSafepointRequested() || - current_state.IsCollectionRequested()); - - if (current_state.IsSafepointRequested()) { - SleepInUnpark(); - continue; - } - - if (current_state.IsCollectionRequested()) { - DCHECK(!current_state.IsSafepointRequested()); - - if (!state_.CompareExchangeStrong(current_state, - current_state.SetRunning())) - continue; - - if (!heap()->ignore_local_gc_requests()) { - heap_->CollectGarbageForBackground(this); - } - - return; - } + DCHECK(current_state.IsCollectionRequested()); + CHECK(state_.CompareExchangeStrong(current_state, + current_state.SetRunning())); + heap_->CollectGarbageForBackground(this); + return; } else { DCHECK(current_state.IsSafepointRequested()); DCHECK(!current_state.IsCollectionRequested()); - - SleepInUnpark(); + TRACE_GC1(heap_->tracer(), GCTracer::Scope::BACKGROUND_UNPARK, + ThreadKind::kBackground); + heap_->safepoint()->WaitInUnpark(); } } } -void LocalHeap::SleepInUnpark() { - GCTracer::Scope::ScopeId scope_id; - ThreadKind thread_kind; - - if (is_main_thread()) { - scope_id = GCTracer::Scope::UNPARK; - thread_kind = ThreadKind::kMain; - } else { - scope_id = GCTracer::Scope::BACKGROUND_UNPARK; - thread_kind = ThreadKind::kBackground; - } - - TRACE_GC1(heap_->tracer(), scope_id, thread_kind); - heap_->safepoint()->WaitInUnpark(); -} - void LocalHeap::EnsureParkedBeforeDestruction() { DCHECK_IMPLIES(!is_main_thread(), IsParked()); } void LocalHeap::SafepointSlowPath() { +#ifdef DEBUG ThreadState current_state = state_.load_relaxed(); DCHECK(current_state.IsRunning()); +#endif if (is_main_thread()) { - DCHECK(current_state.IsSafepointRequested() || - current_state.IsCollectionRequested()); - - if (current_state.IsSafepointRequested()) { - SleepInSafepoint(); - } - - if (current_state.IsCollectionRequested()) { - heap_->CollectGarbageForBackground(this); - } + DCHECK(current_state.IsCollectionRequested()); + heap_->CollectGarbageForBackground(this); } else { DCHECK(current_state.IsSafepointRequested()); DCHECK(!current_state.IsCollectionRequested()); - SleepInSafepoint(); + TRACE_GC1(heap_->tracer(), GCTracer::Scope::BACKGROUND_SAFEPOINT, + ThreadKind::kBackground); + + // Parking the running thread here is an optimization. We do not need to + // wake this thread up to reach the next safepoint. + ThreadState old_state = state_.SetParked(); + CHECK(old_state.IsRunning()); + CHECK(old_state.IsSafepointRequested()); + CHECK(!old_state.IsCollectionRequested()); + + heap_->safepoint()->WaitInSafepoint(); + + Unpark(); } } -void LocalHeap::SleepInSafepoint() { - GCTracer::Scope::ScopeId scope_id; - ThreadKind thread_kind; - - if (is_main_thread()) { - scope_id = GCTracer::Scope::SAFEPOINT; - thread_kind = ThreadKind::kMain; - } else { - scope_id = GCTracer::Scope::BACKGROUND_SAFEPOINT; - thread_kind = ThreadKind::kBackground; - } - - TRACE_GC1(heap_->tracer(), scope_id, thread_kind); - - // Parking the running thread here is an optimization. We do not need to - // wake this thread up to reach the next safepoint. - ThreadState old_state = state_.SetParked(); - CHECK(old_state.IsRunning()); - CHECK(old_state.IsSafepointRequested()); - CHECK_IMPLIES(old_state.IsCollectionRequested(), is_main_thread()); - - heap_->safepoint()->WaitInSafepoint(); - - base::Optional ignore_gc_requests; - if (is_main_thread()) ignore_gc_requests.emplace(heap()); - Unpark(); -} - void LocalHeap::FreeLinearAllocationArea() { old_space_allocator_->FreeLinearAllocationArea(); code_space_allocator_->FreeLinearAllocationArea(); @@ -354,7 +270,7 @@ bool LocalHeap::TryPerformCollection() { return true; } else { DCHECK(IsRunning()); - if (!heap_->collection_barrier_->TryRequestGC()) return false; + heap_->collection_barrier_->RequestGC(); LocalHeap* main_thread = heap_->main_thread_local_heap(); diff --git a/src/heap/local-heap.h b/src/heap/local-heap.h index 35c6af39e7..8ea5a6f336 100644 --- a/src/heap/local-heap.h +++ b/src/heap/local-heap.h @@ -8,7 +8,6 @@ #include #include -#include "src/base/logging.h" #include "src/base/macros.h" #include "src/base/platform/condition-variable.h" #include "src/base/platform/mutex.h" @@ -279,8 +278,6 @@ class V8_EXPORT_PRIVATE LocalHeap { void UnparkSlowPath(); void EnsureParkedBeforeDestruction(); void SafepointSlowPath(); - void SleepInSafepoint(); - void SleepInUnpark(); void EnsurePersistentHandles(); @@ -311,12 +308,10 @@ class V8_EXPORT_PRIVATE LocalHeap { friend class CollectionBarrier; friend class ConcurrentAllocator; - friend class GlobalSafepoint; friend class IsolateSafepoint; friend class Heap; friend class Isolate; friend class ParkedScope; - friend class SafepointScope; friend class UnparkedScope; }; diff --git a/src/heap/parked-scope.h b/src/heap/parked-scope.h index 76d863215e..c7bfa38ce1 100644 --- a/src/heap/parked-scope.h +++ b/src/heap/parked-scope.h @@ -44,8 +44,6 @@ class V8_NODISCARD UnparkedScope { LocalHeap* const local_heap_; }; -// Scope that automatically parks the thread while blocking on the given -// base::Mutex. class V8_NODISCARD ParkedMutexGuard { public: explicit ParkedMutexGuard(LocalIsolate* local_isolate, base::Mutex* mutex) diff --git a/src/heap/safepoint.cc b/src/heap/safepoint.cc index bef9242ffa..d01d7c2a60 100644 --- a/src/heap/safepoint.cc +++ b/src/heap/safepoint.cc @@ -7,9 +7,6 @@ #include #include "src/base/logging.h" -#include "src/base/optional.h" -#include "src/base/platform/mutex.h" -#include "src/common/globals.h" #include "src/handles/handles.h" #include "src/handles/local-handles.h" #include "src/handles/persistent-handles.h" @@ -19,7 +16,6 @@ #include "src/heap/local-heap.h" #include "src/heap/parked-scope.h" #include "src/logging/counters-scopes.h" -#include "src/objects/objects.h" namespace v8 { namespace internal { @@ -27,46 +23,22 @@ namespace internal { IsolateSafepoint::IsolateSafepoint(Heap* heap) : heap_(heap), local_heaps_head_(nullptr), active_safepoint_scopes_(0) {} -void IsolateSafepoint::EnterLocalSafepointScope() { +void IsolateSafepoint::EnterSafepointScope(StopMainThread stop_main_thread) { // Safepoints need to be initiated on the main thread. DCHECK_EQ(ThreadId::Current(), heap_->isolate()->thread_id()); DCHECK_NULL(LocalHeap::Current()); - LockMutex(heap_->isolate()->main_thread_local_heap()); if (++active_safepoint_scopes_ > 1) return; TimedHistogramScope timer( heap_->isolate()->counters()->gc_time_to_safepoint()); TRACE_GC(heap_->tracer(), GCTracer::Scope::TIME_TO_SAFEPOINT); - barrier_.Arm(); - size_t running = SetSafepointRequestedFlags(IncludeMainThread::kNo); - barrier_.WaitUntilRunningThreadsInSafepoint(running); -} - -void IsolateSafepoint::EnterGlobalSafepointScope(Isolate* initiator) { - { - IgnoreLocalGCRequests ignore_gc_requests(initiator->heap()); - LockMutex(initiator->main_thread_local_heap()); - } - CHECK_EQ(active_safepoint_scopes_.exchange(1), 0); + local_heaps_mutex_.Lock(); barrier_.Arm(); - size_t running = - SetSafepointRequestedFlags(IncludeMainThreadUnlessInitiator(initiator)); - barrier_.WaitUntilRunningThreadsInSafepoint(running); -} - -IsolateSafepoint::IncludeMainThread -IsolateSafepoint::IncludeMainThreadUnlessInitiator(Isolate* initiator) { - const bool is_initiator = heap_->isolate() == initiator; - return is_initiator ? IncludeMainThread::kNo : IncludeMainThread::kYes; -} - -size_t IsolateSafepoint::SetSafepointRequestedFlags( - IncludeMainThread include_main_thread) { - size_t running = 0; + int running = 0; // There needs to be at least one LocalHeap for the main thread. DCHECK_NOT_NULL(local_heaps_head_); @@ -74,7 +46,7 @@ size_t IsolateSafepoint::SetSafepointRequestedFlags( for (LocalHeap* local_heap = local_heaps_head_; local_heap; local_heap = local_heap->next_) { if (local_heap->is_main_thread() && - include_main_thread == IncludeMainThread::kNo) { + stop_main_thread == StopMainThread::kNo) { continue; } @@ -87,42 +59,21 @@ size_t IsolateSafepoint::SetSafepointRequestedFlags( CHECK(!old_state.IsSafepointRequested()); } - return running; + barrier_.WaitUntilRunningThreadsInSafepoint(running); } -void IsolateSafepoint::LockMutex(LocalHeap* local_heap) { - if (!local_heaps_mutex_.TryLock()) { - ParkedScope parked_scope(local_heap); - local_heaps_mutex_.Lock(); - } -} +void IsolateSafepoint::LeaveSafepointScope(StopMainThread stop_main_thread) { + // Safepoints need to be initiated on the main thread. + DCHECK_EQ(ThreadId::Current(), heap_->isolate()->thread_id()); + DCHECK_NULL(LocalHeap::Current()); -void IsolateSafepoint::LeaveGlobalSafepointScope(Isolate* initiator) { - local_heaps_mutex_.AssertHeld(); - CHECK_EQ(active_safepoint_scopes_.exchange(0), 1); - ClearSafepointRequestedFlags(IncludeMainThreadUnlessInitiator(initiator)); - barrier_.Disarm(); - local_heaps_mutex_.Unlock(); -} - -void IsolateSafepoint::LeaveLocalSafepointScope() { - local_heaps_mutex_.AssertHeld(); DCHECK_GT(active_safepoint_scopes_, 0); + if (--active_safepoint_scopes_ > 0) return; - if (--active_safepoint_scopes_ == 0) { - ClearSafepointRequestedFlags(IncludeMainThread::kNo); - barrier_.Disarm(); - } - - local_heaps_mutex_.Unlock(); -} - -void IsolateSafepoint::ClearSafepointRequestedFlags( - IncludeMainThread include_main_thread) { for (LocalHeap* local_heap = local_heaps_head_; local_heap; local_heap = local_heap->next_) { if (local_heap->is_main_thread() && - include_main_thread == IncludeMainThread::kNo) { + stop_main_thread == StopMainThread::kNo) { continue; } @@ -134,6 +85,10 @@ void IsolateSafepoint::ClearSafepointRequestedFlags( CHECK_IMPLIES(old_state.IsCollectionRequested(), local_heap->is_main_thread()); } + + barrier_.Disarm(); + + local_heaps_mutex_.Unlock(); } void IsolateSafepoint::WaitInSafepoint() { barrier_.WaitInSafepoint(); } @@ -158,7 +113,7 @@ void IsolateSafepoint::Barrier::Disarm() { } void IsolateSafepoint::Barrier::WaitUntilRunningThreadsInSafepoint( - size_t running) { + int running) { base::MutexGuard guard(&mutex_); DCHECK(IsArmed()); while (stopped_ < running) { @@ -193,8 +148,16 @@ void IsolateSafepoint::Barrier::WaitInUnpark() { } } +SafepointScope::SafepointScope(Heap* heap) : safepoint_(heap->safepoint()) { + safepoint_->EnterSafepointScope(IsolateSafepoint::StopMainThread::kNo); +} + +SafepointScope::~SafepointScope() { + safepoint_->LeaveSafepointScope(IsolateSafepoint::StopMainThread::kNo); +} + bool IsolateSafepoint::ContainsLocalHeap(LocalHeap* local_heap) { - base::RecursiveMutexGuard guard(&local_heaps_mutex_); + base::MutexGuard guard(&local_heaps_mutex_); LocalHeap* current = local_heaps_head_; while (current) { @@ -206,7 +169,7 @@ bool IsolateSafepoint::ContainsLocalHeap(LocalHeap* local_heap) { } bool IsolateSafepoint::ContainsAnyLocalHeap() { - base::RecursiveMutexGuard guard(&local_heaps_mutex_); + base::MutexGuard guard(&local_heaps_mutex_); return local_heaps_head_ != nullptr; } @@ -218,12 +181,6 @@ void IsolateSafepoint::Iterate(RootVisitor* visitor) { } } -SafepointScope::SafepointScope(Heap* heap) : safepoint_(heap->safepoint()) { - safepoint_->EnterLocalSafepointScope(); -} - -SafepointScope::~SafepointScope() { safepoint_->LeaveLocalSafepointScope(); } - GlobalSafepoint::GlobalSafepoint(Isolate* isolate) : shared_isolate_(isolate), shared_heap_(isolate->heap()) {} @@ -247,11 +204,7 @@ void GlobalSafepoint::AppendClient(Isolate* client) { void GlobalSafepoint::RemoveClient(Isolate* client) { DCHECK_EQ(client->heap()->gc_state(), Heap::TEAR_DOWN); - - // A shared heap may have already acquired the client mutex to perform a - // shared GC. We need to park the Isolate here to allow for a shared GC. - IgnoreLocalGCRequests ignore_gc_requests(client->heap()); - ParkedMutexGuard guard(client->main_thread_local_heap(), &clients_mutex_); + base::MutexGuard guard(&clients_mutex_); if (client->global_safepoint_next_client_isolate_) { client->global_safepoint_next_client_isolate_ @@ -275,7 +228,6 @@ void GlobalSafepoint::AssertNoClients() { DCHECK_NULL(clients_head_); } void GlobalSafepoint::EnterGlobalSafepointScope(Isolate* initiator) { if (!clients_mutex_.TryLock()) { - IgnoreLocalGCRequests ignore_gc_requests(initiator->heap()); ParkedScope parked_scope(initiator->main_thread_local_heap()); clients_mutex_.Lock(); } @@ -287,7 +239,9 @@ void GlobalSafepoint::EnterGlobalSafepointScope(Isolate* initiator) { IterateClientIsolates([this, initiator](Isolate* client) { Heap* client_heap = client->heap(); - client_heap->safepoint()->EnterGlobalSafepointScope(initiator); + CHECK_EQ(initiator, client); + client_heap->safepoint()->EnterSafepointScope( + IsolateSafepoint::StopMainThread::kNo); USE(this); DCHECK_EQ(client->shared_isolate(), shared_isolate_); @@ -296,9 +250,10 @@ void GlobalSafepoint::EnterGlobalSafepointScope(Isolate* initiator) { } void GlobalSafepoint::LeaveGlobalSafepointScope(Isolate* initiator) { - IterateClientIsolates([initiator](Isolate* client) { + IterateClientIsolates([](Isolate* client) { Heap* client_heap = client->heap(); - client_heap->safepoint()->LeaveGlobalSafepointScope(initiator); + client_heap->safepoint()->LeaveSafepointScope( + IsolateSafepoint::StopMainThread::kNo); }); clients_mutex_.Unlock(); diff --git a/src/heap/safepoint.h b/src/heap/safepoint.h index b911c2c9ed..e0c19b2a1f 100644 --- a/src/heap/safepoint.h +++ b/src/heap/safepoint.h @@ -7,7 +7,6 @@ #include "src/base/platform/condition-variable.h" #include "src/base/platform/mutex.h" -#include "src/common/globals.h" #include "src/handles/persistent-handles.h" #include "src/heap/local-heap.h" #include "src/objects/visitors.h" @@ -19,12 +18,21 @@ class Heap; class LocalHeap; class RootVisitor; -// Used to bring all threads with heap access in an isolate to a safepoint such -// that e.g. a garbage collection can be performed. +// Used to bring all threads with heap access to a safepoint such that e.g. a +// garbage collection can be performed. class IsolateSafepoint final { public: explicit IsolateSafepoint(Heap* heap); + // Wait until unpark operation is safe again + void WaitInUnpark(); + + // Enter the safepoint from a running thread + void WaitInSafepoint(); + + // Running thread reached a safepoint by parking itself. + void NotifyPark(); + V8_EXPORT_PRIVATE bool ContainsLocalHeap(LocalHeap* local_heap); V8_EXPORT_PRIVATE bool ContainsAnyLocalHeap(); @@ -50,7 +58,7 @@ class IsolateSafepoint final { base::ConditionVariable cv_stopped_; bool armed_; - size_t stopped_ = 0; + int stopped_ = 0; bool IsArmed() { return armed_; } @@ -59,42 +67,23 @@ class IsolateSafepoint final { void Arm(); void Disarm(); - void WaitUntilRunningThreadsInSafepoint(size_t running); + void WaitUntilRunningThreadsInSafepoint(int running); void WaitInSafepoint(); void WaitInUnpark(); void NotifyPark(); }; - enum class IncludeMainThread { kYes, kNo }; + enum class StopMainThread { kYes, kNo }; - // Wait until unpark operation is safe again. - void WaitInUnpark(); - - // Enter the safepoint from a running thread. - void WaitInSafepoint(); - - // Running thread reached a safepoint by parking itself. - void NotifyPark(); - - void EnterLocalSafepointScope(); - void EnterGlobalSafepointScope(Isolate* initiator); - - void LeaveLocalSafepointScope(); - void LeaveGlobalSafepointScope(Isolate* initiator); - - IncludeMainThread IncludeMainThreadUnlessInitiator(Isolate* initiator); - - void LockMutex(LocalHeap* local_heap); - - size_t SetSafepointRequestedFlags(IncludeMainThread include_main_thread); - void ClearSafepointRequestedFlags(IncludeMainThread include_main_thread); + void EnterSafepointScope(StopMainThread stop_main_thread); + void LeaveSafepointScope(StopMainThread stop_main_thread); template void AddLocalHeap(LocalHeap* local_heap, Callback callback) { // Safepoint holds this lock in order to stop threads from starting or // stopping. - base::RecursiveMutexGuard guard(&local_heaps_mutex_); + base::MutexGuard guard(&local_heaps_mutex_); // Additional code protected from safepoint callback(); @@ -108,7 +97,7 @@ class IsolateSafepoint final { template void RemoveLocalHeap(LocalHeap* local_heap, Callback callback) { - base::RecursiveMutexGuard guard(&local_heaps_mutex_); + base::MutexGuard guard(&local_heaps_mutex_); // Additional code protected from safepoint callback(); @@ -124,12 +113,10 @@ class IsolateSafepoint final { Barrier barrier_; Heap* heap_; - // Mutex is used both for safepointing and adding/removing threads. A - // RecursiveMutex is needed since we need to support nested SafepointScopes. - base::RecursiveMutex local_heaps_mutex_; + base::Mutex local_heaps_mutex_; LocalHeap* local_heaps_head_; - std::atomic active_safepoint_scopes_; + int active_safepoint_scopes_; friend class Heap; friend class GlobalSafepoint; diff --git a/src/init/heap-symbols.h b/src/init/heap-symbols.h index 34a4bb18e4..52bbc87b20 100644 --- a/src/init/heap-symbols.h +++ b/src/init/heap-symbols.h @@ -575,7 +575,6 @@ F(MINOR_MC_MARKING_DEQUE) \ F(MINOR_MC_RESET_LIVENESS) \ F(MINOR_MC_SWEEPING) \ - F(SAFEPOINT) \ F(SCAVENGER) \ F(SCAVENGER_COMPLETE_SWEEP_ARRAY_BUFFERS) \ F(SCAVENGER_FAST_PROMOTE) \ @@ -592,8 +591,7 @@ F(SCAVENGER_SWEEP_ARRAY_BUFFERS) \ F(TIME_TO_GLOBAL_SAFEPOINT) \ F(TIME_TO_SAFEPOINT) \ - F(UNMAPPER) \ - F(UNPARK) + F(UNMAPPER) #define TRACER_BACKGROUND_SCOPES(F) \ F(BACKGROUND_YOUNG_ARRAY_BUFFER_SWEEP) \ diff --git a/test/cctest/heap/test-shared-heap.cc b/test/cctest/heap/test-shared-heap.cc index 005a1139f2..483e67a70d 100644 --- a/test/cctest/heap/test-shared-heap.cc +++ b/test/cctest/heap/test-shared-heap.cc @@ -164,24 +164,22 @@ UNINITIALIZED_TEST(SharedCollectionWithoutClients) { Isolate::Delete(shared_isolate); } -void AllocateInSharedHeap(Isolate* shared_isolate, int iterations = 100) { +void AllocateInSharedHeap(Isolate* shared_isolate) { SetupClientIsolateAndRunCallback( shared_isolate, - [iterations](v8::Isolate* client_isolate, Isolate* i_client_isolate) { + [](v8::Isolate* client_isolate, Isolate* i_client_isolate) { HandleScope scope(i_client_isolate); std::vector> arrays; const int kKeptAliveArrays = 1000; - for (int i = 0; i < kNumIterations * iterations; i++) { - HandleScope scope(i_client_isolate); + for (int i = 0; i < kNumIterations * 100; i++) { + HandleScope new_scope(i_client_isolate); Handle array = i_client_isolate->factory()->NewFixedArray( 100, AllocationType::kSharedOld); if (i < kKeptAliveArrays) { // Keep some of those arrays alive across GCs. - arrays.push_back(scope.CloseAndEscape(array)); + arrays.push_back(new_scope.CloseAndEscape(array)); } - i_client_isolate->factory()->NewFixedArray(100, - AllocationType::kYoung); } for (Handle array : arrays) { @@ -205,46 +203,5 @@ UNINITIALIZED_TEST(SharedCollectionWithOneClient) { Isolate::Delete(shared_isolate); } -namespace { -class SharedFixedArrayAllocationThread final : public v8::base::Thread { - public: - explicit SharedFixedArrayAllocationThread(Isolate* shared) - : v8::base::Thread( - base::Thread::Options("SharedFixedArrayAllocationThread")), - shared_(shared) {} - - void Run() override { AllocateInSharedHeap(shared_, 5); } - - Isolate* shared_; -}; -} // namespace - -UNINITIALIZED_TEST(SharedCollectionWithMultipleClients) { - FLAG_max_old_space_size = 8; - if (!ReadOnlyHeap::IsReadOnlySpaceShared()) return; - std::unique_ptr allocator( - v8::ArrayBuffer::Allocator::NewDefaultAllocator()); - - v8::Isolate::CreateParams create_params; - create_params.array_buffer_allocator = allocator.get(); - Isolate* shared_isolate = Isolate::NewShared(create_params); - - std::vector> threads; - const int kThreads = 4; - - for (int i = 0; i < kThreads; i++) { - auto thread = - std::make_unique(shared_isolate); - CHECK(thread->Start()); - threads.push_back(std::move(thread)); - } - - for (auto& thread : threads) { - thread->Join(); - } - - Isolate::Delete(shared_isolate); -} - } // namespace internal } // namespace v8