Skip to content

Fix concurrent sparkplug #47461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
targos wants to merge 5 commits into from
Closed

Fix concurrent sparkplug #47461

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
20a5e25
deps: V8: cherry-pick 8f61b4133805
targos Apr 7, 2023
8c5b8de
deps: V8: cherry-pick f820e706a0b9
targos Apr 7, 2023
776a1cf
src: do not drain worker tasks on main thread
Apr 6, 2023
067a20d
Revert "test: skip test-wasm-web-api on ARM"
targos Apr 6, 2023
81b1a41
Revert "test: skip instantiateStreaming-bad-imports WPT"
targos Apr 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
deps: V8: cherry-pick 8f61b4133805 
Original commit message:

    [heap] Cache ForegroundTaskRunner in CollectionBarrier

    The API doesn't allow us to invoke GetForegroundTaskRunner() from
    a background thread. We need to cache the task runner in the ctor
    of CollectionBarrier during heap setup.

    We can't create the CollectionBarrier inside Heap's ctor anymore
    because at that point the TaskRunner might not be set up (e.g. Node).

    Bug: v8:13902
    Change-Id: I814b9dbecb0ded673108bfb7b730e7c9338cf5f5
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4403395
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#86981}

Refs: v8/v8@8f61b41
  • Loading branch information
@targos
targos committed Apr 7, 2023
commit 20a5e25ec24d7d6f22ddf7d1246eac2183b501df
2 changes: 1 addition & 1 deletion common.gypi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.3',
'v8_embedder_string': '-node.4',

##### V8 defaults for Node.js #####

Expand Down
24 changes: 13 additions & 11 deletions deps/v8/src/heap/collection-barrier.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
namespace v8 {
namespace internal {

CollectionBarrier::CollectionBarrier(Heap* heap)
: heap_(heap),
foreground_task_runner_(V8::GetCurrentPlatform()->GetForegroundTaskRunner(
reinterpret_cast<v8::Isolate*>(heap->isolate()))) {}

bool CollectionBarrier::WasGCRequested() {
return collection_requested_.load();
}
Expand Down Expand Up @@ -95,7 +100,14 @@ bool CollectionBarrier::AwaitCollectionBackground(LocalHeap* local_heap) {
}

// The first thread needs to activate the stack guard and post the task.
if (first_thread) ActivateStackGuardAndPostTask();
if (first_thread) {
Isolate* isolate = heap_->isolate();
ExecutionAccess access(isolate);
isolate->stack_guard()->RequestGC();

foreground_task_runner_->PostTask(
std::make_unique<BackgroundCollectionInterruptTask>(heap_));
}

ParkedScope scope(local_heap);
base::MutexGuard guard(&mutex_);
Expand All @@ -109,16 +121,6 @@ bool CollectionBarrier::AwaitCollectionBackground(LocalHeap* local_heap) {
return collection_performed_;
}

void CollectionBarrier::ActivateStackGuardAndPostTask() {
Isolate* isolate = heap_->isolate();
ExecutionAccess access(isolate);
isolate->stack_guard()->RequestGC();

V8::GetCurrentPlatform()
->GetForegroundTaskRunner(reinterpret_cast<v8::Isolate*>(isolate))
->PostTask(std::make_unique<BackgroundCollectionInterruptTask>(heap_));
}

void CollectionBarrier::StopTimeToCollectionTimer() {
if (collection_requested_.load()) {
base::MutexGuard guard(&mutex_);
Expand Down
8 changes: 4 additions & 4 deletions deps/v8/src/heap/collection-barrier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Heap;
// This class stops and resumes all background threads waiting for GC.
class CollectionBarrier {
public:
explicit CollectionBarrier(Heap* heap) : heap_(heap) {}
explicit CollectionBarrier(Heap* heap);

// Returns true when collection was requested.
bool WasGCRequested();
Expand All @@ -49,9 +49,6 @@ class CollectionBarrier {
bool AwaitCollectionBackground(LocalHeap* local_heap);

private:
// Activate stack guards and posting a task to perform the GC.
void ActivateStackGuardAndPostTask();

Heap* heap_;
base::Mutex mutex_;
base::ConditionVariable cv_wakeup_;
Expand All @@ -72,6 +69,9 @@ class CollectionBarrier {

// Will be set as soon as Isolate starts tear down.
bool shutdown_requested_ = false;

// Used to post tasks on the main thread.
std::shared_ptr<v8::TaskRunner> foreground_task_runner_;
};

} // namespace internal
Expand Down
3 changes: 2 additions & 1 deletion deps/v8/src/heap/heap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ Heap::Heap()
allocation_type_for_in_place_internalizable_strings_(
isolate()->OwnsStringTables() ? AllocationType::kOld
: AllocationType::kSharedOld),
collection_barrier_(new CollectionBarrier(this)),
marking_state_(isolate_),
non_atomic_marking_state_(isolate_),
atomic_marking_state_(isolate_),
Expand Down Expand Up @@ -5431,6 +5430,8 @@ void Heap::SetUp(LocalHeap* main_thread_local_heap) {
code_page_allocator = isolate_->page_allocator();
}

collection_barrier_.reset(new CollectionBarrier(this));

// Set up memory allocator.
memory_allocator_.reset(
new MemoryAllocator(isolate_, code_page_allocator, MaxReserved()));
Expand Down