Commit d79e0c4f authored by Dominik Inführ's avatar Dominik Inführ Committed by V8 LUCI CQ

[heap] Add and remove allocation observers without using safepoints

Safepoints were used in Heap::AddAllocationObserversToAllSpaces and
Heap::RemoveAllocationObserversFromAllSpaces as a poor man's approach
to synchronization. This CL removes the safepoint and protects the
potential race on the free list with a mutex in
PagedSpace::DecreaseLimit.

The motivation for this CL is that SafepointScope might possibly park
the main thread in the future. However parking is only allowed if GCs
are also allowed. GCs are not allowed when running allocation observers,
so an allocation observer would not be able to add or remove additional
observers as is currently done in StressConcurrentAllocationObserver.

Also adding additional checks to the safepoint to ensure that we are
on the main thread.

Bug: v8:11708
Change-Id: I4e65a83ac4015d30b15d8c4eeaed4ea759b7c982
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3160523
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76867}
parent d407af0d
......@@ -1003,7 +1003,6 @@ void Heap::MergeAllocationSitePretenuringFeedback(
void Heap::AddAllocationObserversToAllSpaces(
AllocationObserver* observer, AllocationObserver* new_space_observer) {
DCHECK(observer && new_space_observer);
SafepointScope scope(this);
for (SpaceIterator it(this); it.HasNext();) {
Space* space = it.Next();
......@@ -1018,7 +1017,6 @@ void Heap::AddAllocationObserversToAllSpaces(
void Heap::RemoveAllocationObserversFromAllSpaces(
AllocationObserver* observer, AllocationObserver* new_space_observer) {
DCHECK(observer && new_space_observer);
SafepointScope scope(this);
for (SpaceIterator it(this); it.HasNext();) {
Space* space = it.Next();
......
......@@ -365,6 +365,7 @@ void PagedSpace::DecreaseLimit(Address new_limit) {
optional_scope.emplace(chunk);
}
ConcurrentAllocationMutex guard(this);
SetTopAndLimit(top(), new_limit);
Free(new_limit, old_limit - new_limit,
SpaceAccountingMode::kSpaceAccounted);
......
......@@ -23,6 +23,10 @@ GlobalSafepoint::GlobalSafepoint(Heap* heap)
: heap_(heap), local_heaps_head_(nullptr), active_safepoint_scopes_(0) {}
void GlobalSafepoint::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());
if (++active_safepoint_scopes_ > 1) return;
TimedHistogramScope timer(
......@@ -32,7 +36,6 @@ void GlobalSafepoint::EnterSafepointScope(StopMainThread stop_main_thread) {
local_heaps_mutex_.Lock();
barrier_.Arm();
DCHECK_NULL(LocalHeap::Current());
int running = 0;
......@@ -66,11 +69,13 @@ void GlobalSafepoint::EnterSafepointScope(StopMainThread stop_main_thread) {
}
void GlobalSafepoint::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());
DCHECK_GT(active_safepoint_scopes_, 0);
if (--active_safepoint_scopes_ > 0) return;
DCHECK_NULL(LocalHeap::Current());
for (LocalHeap* local_heap = local_heaps_head_; local_heap;
local_heap = local_heap->next_) {
if (local_heap->is_main_thread() &&
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment