Skip to content

Commit bd0d029

Browse files
aamCommit Queue
authored andcommitted
[vm/shared] Remove "single mutator" fastpath from RunWithStoppedMutators.
In presence of isolate-less mutators, the check for "single mutator"-scenario is not-trivial, acquiring active_mutator_count mutex results in circular mutex dependencies. BUG=#60857 BUG=#60953 TEST=ci Change-Id: I5a12f9b8d4bc54042137012668ed1afee7c7739f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/435241 Reviewed-by: Ryan Macnak <[email protected]> Commit-Queue: Alexander Aprelev <[email protected]>
1 parent 29698c7 commit bd0d029

File tree

5 files changed

+19
-41
lines changed

5 files changed

+19
-41
lines changed

runtime/lib/concurrent.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ DEFINE_FFI_NATIVE_ENTRY(IsolateGroup_runSync,
145145
Thread::EnterIsolate(saved_isolate);
146146

147147
Thread* T = Thread::Current();
148-
T->EnterSafepointToNative();
149148
T->set_execution_state(Thread::kThreadInNative);
149+
T->EnterSafepoint();
150150

151151
Dart_Handle local_handle = Dart_HandleFromPersistent(persistent_result);
152152
Dart_DeletePersistentHandle(persistent_result);

runtime/vm/dart_api_impl.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1554,8 +1554,8 @@ DART_EXPORT void Dart_EnterIsolate(Dart_Isolate isolate) {
15541554
// TransitionXXX scope objects as the reverse transition happens
15551555
// outside this scope in Dart_ExitIsolate/Dart_ShutdownIsolate.
15561556
Thread* T = Thread::Current();
1557-
T->EnterSafepointToNative();
15581557
T->set_execution_state(Thread::kThreadInNative);
1558+
T->EnterSafepointToNative();
15591559
}
15601560

15611561
DART_EXPORT void Dart_StartProfiling() {

runtime/vm/isolate.cc

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2909,37 +2909,27 @@ Isolate* IsolateGroup::FirstIsolateLocked() const {
29092909
return isolates_.IsEmpty() ? nullptr : isolates_.First();
29102910
}
29112911

2912-
void IsolateGroup::RunWithStoppedMutatorsCallable(
2913-
Callable* single_current_mutator,
2914-
Callable* otherwise,
2915-
bool use_force_growth_in_otherwise) {
2912+
void IsolateGroup::RunWithStoppedMutatorsCallable(Callable* callable,
2913+
bool use_force_growth) {
29162914
auto thread = Thread::Current();
29172915
StoppedMutatorsScope stopped_mutators_scope(thread);
29182916

29192917
if (thread->OwnsSafepoint()) {
29202918
RELEASE_ASSERT(thread->OwnsSafepoint());
2921-
single_current_mutator->Call();
2919+
callable->Call();
29222920
return;
29232921
}
29242922

2925-
{
2926-
SafepointReadRwLocker ml(thread, isolates_lock_.get());
2927-
if (thread->IsDartMutatorThread() && ContainsOnlyOneIsolate()) {
2928-
single_current_mutator->Call();
2929-
return;
2930-
}
2931-
}
2932-
29332923
// We use the more strict safepoint operation scope here (which ensures that
29342924
// all other threads, including auxiliary threads are at a safepoint), even
29352925
// though we only need to ensure that the mutator threads are stopped.
2936-
if (use_force_growth_in_otherwise) {
2926+
if (use_force_growth) {
29372927
ForceGrowthSafepointOperationScope safepoint_scope(
29382928
thread, SafepointLevel::kGCAndDeopt);
2939-
otherwise->Call();
2929+
callable->Call();
29402930
} else {
29412931
DeoptSafepointOperationScope safepoint_scope(thread);
2942-
otherwise->Call();
2932+
callable->Call();
29432933
}
29442934
}
29452935

runtime/vm/isolate.h

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ class IsolateProfilerData;
6161
class Log;
6262
class Message;
6363
class MessageHandler;
64-
class MonitorLocker;
6564
class Mutex;
6665
class Object;
6766
class ObjectIdRing;
@@ -599,32 +598,20 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
599598
// Ensures mutators are stopped during execution of the provided function.
600599
//
601600
// If the current thread is the only mutator in the isolate group,
602-
// [single_current_mutator] will be called. Otherwise [otherwise] will be
601+
// [callable] will be called directly. Otherwise [callable] will be
603602
// called inside a [SafepointOperationsScope] (or
604-
// [ForceGrowthSafepointOperationScope] if [use_force_growth_in_otherwise]
603+
// [ForceGrowthSafepointOperationScope] if [use_force_growth]
605604
// is set).
606605
//
607606
// During the duration of this function, no new isolates can be added to the
608607
// isolate group.
609-
void RunWithStoppedMutatorsCallable(
610-
Callable* single_current_mutator,
611-
Callable* otherwise,
612-
bool use_force_growth_in_otherwise = false);
613-
614-
template <typename T, typename S>
615-
void RunWithStoppedMutators(T single_current_mutator,
616-
S otherwise,
617-
bool use_force_growth_in_otherwise = false) {
618-
LambdaCallable<T> single_callable(single_current_mutator);
619-
LambdaCallable<S> otherwise_callable(otherwise);
620-
RunWithStoppedMutatorsCallable(&single_callable, &otherwise_callable,
621-
use_force_growth_in_otherwise);
622-
}
608+
void RunWithStoppedMutatorsCallable(Callable* callable,
609+
bool use_force_growth = false);
623610

624611
template <typename T>
625612
void RunWithStoppedMutators(T function, bool use_force_growth = false) {
626613
LambdaCallable<T> callable(function);
627-
RunWithStoppedMutatorsCallable(&callable, &callable, use_force_growth);
614+
RunWithStoppedMutatorsCallable(&callable, use_force_growth);
628615
}
629616

630617
#ifndef PRODUCT

runtime/vm/thread.cc

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,10 @@ void Thread::ExitIsolateGroupAsHelper(bool bypass_safepoint) {
520520

521521
void Thread::EnterIsolateGroupAsMutator(IsolateGroup* isolate_group,
522522
bool bypass_safepoint) {
523+
isolate_group->IncreaseMutatorCount(/*thread=*/nullptr,
524+
/*is_nested_reenter=*/true,
525+
/*was_stolen=*/false);
526+
523527
Thread* thread = AddActiveThread(isolate_group, /*isolate=*/nullptr,
524528
kMutatorTask, bypass_safepoint);
525529
RELEASE_ASSERT(thread != nullptr);
@@ -542,10 +546,6 @@ void Thread::EnterIsolateGroupAsMutator(IsolateGroup* isolate_group,
542546
thread->SetStackLimit(OSThread::Current()->overflow_stack_limit());
543547
#endif
544548

545-
isolate_group->IncreaseMutatorCount(/*thread=*/thread,
546-
/*is_nested_reenter=*/false,
547-
/*was_stolen=*/true);
548-
549549
thread->AssertDartMutatorInvariants();
550550
}
551551

@@ -559,8 +559,9 @@ void Thread::ExitIsolateGroupAsMutator(bool bypass_safepoint) {
559559
thread->ResetMutatorState();
560560
thread->ClearStackLimit();
561561
SuspendThreadInternal(thread, VMTag::kInvalidTagId);
562-
thread->isolate_group()->DecreaseMutatorCount(/*is_nested_exit=*/true);
562+
auto group = thread->isolate_group();
563563
FreeActiveThread(thread, /*isolate=*/nullptr, bypass_safepoint);
564+
group->DecreaseMutatorCount(/*is_nested_exit=*/true);
564565
}
565566

566567
void Thread::EnterIsolateGroupAsNonMutator(IsolateGroup* isolate_group,

0 commit comments

Comments
 (0)