From 1a6755cbf4b0c46a4f944f25861495bde37f9fca Mon Sep 17 00:00:00 2001 From: Alexander Aprelev Date: Thu, 24 Apr 2025 08:33:05 -0700 Subject: [PATCH] [vm] Clean up bool return value for EnterIsolate methods. Effectively these methods always succeed, so return value checking just can be source of confusion. TEST=ci Change-Id: I0a93f130b03c0f66be733c939a1d795d704291d1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/423963 Commit-Queue: Alexander Aprelev Reviewed-by: Slava Egorov --- runtime/lib/isolate.cc | 5 ++--- runtime/vm/compiler/jit/compiler.cc | 5 ++--- runtime/vm/heap/marker.cc | 5 ++--- runtime/vm/heap/safepoint.cc | 5 ++--- runtime/vm/heap/sweeper.cc | 4 +--- runtime/vm/isolate.h | 5 ++--- runtime/vm/thread.cc | 32 ++++++++++++----------------- runtime/vm/thread.h | 4 ++-- 8 files changed, 26 insertions(+), 39 deletions(-) diff --git a/runtime/lib/isolate.cc b/runtime/lib/isolate.cc index 87e218191ec..c7f1161378a 100644 --- a/runtime/lib/isolate.cc +++ b/runtime/lib/isolate.cc @@ -1025,9 +1025,8 @@ class SpawnIsolateTask : public ThreadPool::Task { } else if (state_->isolate_group() != nullptr) { ASSERT(IsolateGroup::Current() == nullptr); const bool kBypassSafepoint = false; - const bool result = Thread::EnterIsolateGroupAsHelper( - state_->isolate_group(), Thread::kSpawnTask, kBypassSafepoint); - ASSERT(result); + Thread::EnterIsolateGroupAsHelper(state_->isolate_group(), + Thread::kSpawnTask, kBypassSafepoint); state_ = nullptr; Thread::ExitIsolateGroupAsHelper(kBypassSafepoint); } else { diff --git a/runtime/vm/compiler/jit/compiler.cc b/runtime/vm/compiler/jit/compiler.cc index dee1a6bd843..117911180bb 100644 --- a/runtime/vm/compiler/jit/compiler.cc +++ b/runtime/vm/compiler/jit/compiler.cc @@ -1094,9 +1094,8 @@ BackgroundCompiler::~BackgroundCompiler() { } void BackgroundCompiler::Run() { - bool result = Thread::EnterIsolateGroupAsHelper( - isolate_group_, Thread::kCompilerTask, /*bypass_safepoint=*/false); - ASSERT(result); + Thread::EnterIsolateGroupAsHelper(isolate_group_, Thread::kCompilerTask, + /*bypass_safepoint=*/false); { Thread* thread = Thread::Current(); StackZone stack_zone(thread); diff --git a/runtime/vm/heap/marker.cc b/runtime/vm/heap/marker.cc index 5bd714b3728..98b7d12d1da 100644 --- a/runtime/vm/heap/marker.cc +++ b/runtime/vm/heap/marker.cc @@ -1044,9 +1044,8 @@ class ConcurrentMarkTask : public ThreadPool::Task { } virtual void Run() { - bool result = Thread::EnterIsolateGroupAsHelper( - isolate_group_, Thread::kMarkerTask, /*bypass_safepoint=*/true); - ASSERT(result); + Thread::EnterIsolateGroupAsHelper(isolate_group_, Thread::kMarkerTask, + /*bypass_safepoint=*/true); { TIMELINE_FUNCTION_GC_DURATION(Thread::Current(), "ConcurrentMark"); int64_t start = OS::GetCurrentMonotonicMicros(); diff --git a/runtime/vm/heap/safepoint.cc b/runtime/vm/heap/safepoint.cc index 48be32f665b..2b25c7b65fb 100644 --- a/runtime/vm/heap/safepoint.cc +++ b/runtime/vm/heap/safepoint.cc @@ -437,9 +437,8 @@ void SafepointTask::Run() { return; } - bool result = Thread::EnterIsolateGroupAsHelper(isolate_group_, kind_, - /*bypass_safepoint=*/true); - ASSERT(result); + Thread::EnterIsolateGroupAsHelper(isolate_group_, kind_, + /*bypass_safepoint=*/true); RunEnteredIsolateGroup(); diff --git a/runtime/vm/heap/sweeper.cc b/runtime/vm/heap/sweeper.cc index b4d4b61d84a..2638204c29e 100644 --- a/runtime/vm/heap/sweeper.cc +++ b/runtime/vm/heap/sweeper.cc @@ -176,9 +176,7 @@ class ConcurrentSweeperTask : public ThreadPool::Task { } virtual void Run() { - bool result = Thread::EnterIsolateGroupAsNonMutator(isolate_group_, - Thread::kSweeperTask); - ASSERT(result); + Thread::EnterIsolateGroupAsNonMutator(isolate_group_, Thread::kSweeperTask); PageSpace* old_space = isolate_group_->heap()->old_space(); { Thread* thread = Thread::Current(); diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index 8272df75cf1..a210c37decc 100644 --- a/runtime/vm/isolate.h +++ b/runtime/vm/isolate.h @@ -1804,9 +1804,8 @@ class EnterIsolateGroupScope { explicit EnterIsolateGroupScope(IsolateGroup* isolate_group) : isolate_group_(isolate_group) { ASSERT(IsolateGroup::Current() == nullptr); - const bool result = Thread::EnterIsolateGroupAsHelper( - isolate_group_, Thread::kUnknownTask, /*bypass_safepoint=*/false); - ASSERT(result); + Thread::EnterIsolateGroupAsHelper(isolate_group_, Thread::kUnknownTask, + /*bypass_safepoint=*/false); } ~EnterIsolateGroupScope() { diff --git a/runtime/vm/thread.cc b/runtime/vm/thread.cc index c67548d89b2..68de665e252 100644 --- a/runtime/vm/thread.cc +++ b/runtime/vm/thread.cc @@ -482,22 +482,19 @@ void Thread::ExitIsolate(bool isolate_shutdown) { } } -bool Thread::EnterIsolateGroupAsHelper(IsolateGroup* isolate_group, +void Thread::EnterIsolateGroupAsHelper(IsolateGroup* isolate_group, TaskKind kind, bool bypass_safepoint) { Thread* thread = AddActiveThread(isolate_group, /*isolate=*/nullptr, /*is_dart_mutator=*/false, bypass_safepoint); - if (thread != nullptr) { - thread->SetupState(kind); - // Even if [bypass_safepoint] is true, a thread may need mutator state (e.g. - // parallel scavenger threads write to the [Thread]s storebuffer) - thread->SetupMutatorState(kind); - ResumeThreadInternal(thread); + RELEASE_ASSERT(thread != nullptr); + thread->SetupState(kind); + // Even if [bypass_safepoint] is true, a thread may need mutator state (e.g. + // parallel scavenger threads write to the [Thread]s storebuffer) + thread->SetupMutatorState(kind); + ResumeThreadInternal(thread); - thread->AssertNonDartMutatorInvariants(); - return true; - } - return false; + thread->AssertNonDartMutatorInvariants(); } void Thread::ExitIsolateGroupAsHelper(bool bypass_safepoint) { @@ -513,19 +510,16 @@ void Thread::ExitIsolateGroupAsHelper(bool bypass_safepoint) { bypass_safepoint); } -bool Thread::EnterIsolateGroupAsNonMutator(IsolateGroup* isolate_group, +void Thread::EnterIsolateGroupAsNonMutator(IsolateGroup* isolate_group, TaskKind kind) { Thread* thread = AddActiveThread(isolate_group, /*isolate=*/nullptr, /*is_dart_mutator=*/false, /*bypass_safepoint=*/true); - if (thread != nullptr) { - thread->SetupState(kind); - ResumeThreadInternal(thread); + ASSERT(thread != nullptr); + thread->SetupState(kind); + ResumeThreadInternal(thread); - thread->AssertNonMutatorInvariants(); - return true; - } - return false; + thread->AssertNonMutatorInvariants(); } void Thread::ExitIsolateGroupAsNonMutator() { diff --git a/runtime/vm/thread.h b/runtime/vm/thread.h index 0a82fa284ff..64d2bd1f7b7 100644 --- a/runtime/vm/thread.h +++ b/runtime/vm/thread.h @@ -387,12 +387,12 @@ class Thread : public ThreadState { // Makes the current thread exit its isolate. static void ExitIsolate(bool isolate_shutdown = false); - static bool EnterIsolateGroupAsHelper(IsolateGroup* isolate_group, + static void EnterIsolateGroupAsHelper(IsolateGroup* isolate_group, TaskKind kind, bool bypass_safepoint); static void ExitIsolateGroupAsHelper(bool bypass_safepoint); - static bool EnterIsolateGroupAsNonMutator(IsolateGroup* isolate_group, + static void EnterIsolateGroupAsNonMutator(IsolateGroup* isolate_group, TaskKind kind); static void ExitIsolateGroupAsNonMutator();