Skip to content

Commit c47a8b0

Browse files
aamCommit Queue
authored andcommitted
[vm/shared] Restore fast-path for RunWithStoppedMutators.
The fastpath was removed in https://dart.googlesource.com/sdk.git/+/bd0d029792bce2bc4b0b7fa1659d24350266b3d1 because the fastpath didn't take into account isolategroup-shared mutators and holding an active_mutators_monitor_ caused tsan-identified potential circular locking. Removal actually resulted in regressions in flutter reassemble benchmarks. This PR restores fastpath and instead of holding active_mutators_monitor_ it just keeps track of isolategroup-shared mutators counter, which this PR adds as well. BUG=flutter/flutter#171722 TEST=ci, manual patch into flutter and run reload Change-Id: I00e01d930788697c8ae91fda73b09e013e718fad Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/439802 Commit-Queue: Alexander Aprelev <[email protected]> Reviewed-by: Slava Egorov <[email protected]>
1 parent 3e462c1 commit c47a8b0

File tree

3 files changed

+25
-2
lines changed

3 files changed

+25
-2
lines changed

runtime/vm/isolate.cc

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,8 @@ bool IsolateGroup::ContainsOnlyOneIsolate() {
451451
// We do allow 0 here as well, because the background compiler might call
452452
// this method while the mutator thread is in shutdown procedure and
453453
// unregistered itself already.
454-
return isolate_count_ == 0 || isolate_count_ == 1;
454+
return (isolate_count_ == 0 || isolate_count_ == 1) &&
455+
group_mutator_count_ == 0;
455456
}
456457

457458
void IsolateGroup::UnregisterIsolate(Isolate* isolate) {
@@ -470,6 +471,16 @@ bool IsolateGroup::UnregisterIsolateDecrementCount() {
470471
return isolate_count_ == 0;
471472
}
472473

474+
void IsolateGroup::RegisterIsolateGroupMutator() {
475+
SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
476+
group_mutator_count_++;
477+
}
478+
479+
void IsolateGroup::UnregisterIsolateGroupMutator() {
480+
SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
481+
group_mutator_count_--;
482+
}
483+
473484
void IsolateGroup::CreateHeap(bool is_vm_isolate,
474485
bool is_service_or_kernel_isolate) {
475486
Heap::Init(this, is_vm_isolate,
@@ -2917,6 +2928,14 @@ void IsolateGroup::RunWithStoppedMutatorsCallable(Callable* callable) {
29172928
return;
29182929
}
29192930

2931+
{
2932+
SafepointReadRwLocker ml(thread, isolates_lock_.get());
2933+
if (thread->IsDartMutatorThread() && ContainsOnlyOneIsolate()) {
2934+
callable->Call();
2935+
return;
2936+
}
2937+
}
2938+
29202939
// We use the more strict safepoint operation scope here (which ensures that
29212940
// all other threads, including auxiliary threads are at a safepoint), even
29222941
// though we only need to ensure that the mutator threads are stopped.

runtime/vm/isolate.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,12 +369,13 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
369369

370370
IdleTimeHandler* idle_time_handler() { return &idle_time_handler_; }
371371

372-
// Returns true if this is the first isolate registered.
373372
void RegisterIsolate(Isolate* isolate);
374373
void UnregisterIsolate(Isolate* isolate);
375374
// Returns `true` if this was the last isolate and the caller is responsible
376375
// for deleting the isolate group.
377376
bool UnregisterIsolateDecrementCount();
377+
void RegisterIsolateGroupMutator();
378+
void UnregisterIsolateGroupMutator();
378379

379380
bool ContainsOnlyOneIsolate();
380381

@@ -902,6 +903,7 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
902903
IntrusiveDList<Isolate> isolates_;
903904
RelaxedAtomic<Dart_Port> interrupt_port_ = ILLEGAL_PORT;
904905
intptr_t isolate_count_ = 0;
906+
intptr_t group_mutator_count_ = 0;
905907
bool initial_spawn_successful_ = false;
906908
Dart_LibraryTagHandler library_tag_handler_ = nullptr;
907909
Dart_DeferredLoadHandler deferred_load_handler_ = nullptr;

runtime/vm/thread.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,7 @@ void Thread::EnterIsolateGroupAsMutator(IsolateGroup* isolate_group,
523523
isolate_group->IncreaseMutatorCount(/*thread=*/nullptr,
524524
/*is_nested_reenter=*/true,
525525
/*was_stolen=*/false);
526+
isolate_group->RegisterIsolateGroupMutator();
526527

527528
Thread* thread = AddActiveThread(isolate_group, /*isolate=*/nullptr,
528529
kMutatorTask, bypass_safepoint);
@@ -561,6 +562,7 @@ void Thread::ExitIsolateGroupAsMutator(bool bypass_safepoint) {
561562
SuspendThreadInternal(thread, VMTag::kInvalidTagId);
562563
auto group = thread->isolate_group();
563564
FreeActiveThread(thread, /*isolate=*/nullptr, bypass_safepoint);
565+
group->UnregisterIsolateGroupMutator();
564566
group->DecreaseMutatorCount(/*is_nested_exit=*/true);
565567
}
566568

0 commit comments

Comments
 (0)