Skip to content

Commit 1a6755c

Browse files
aamCommit Queue
authored and
Commit Queue
committed
[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 <[email protected]> Reviewed-by: Slava Egorov <[email protected]>
1 parent e81bc24 commit 1a6755c

File tree

8 files changed

+26
-39
lines changed

8 files changed

+26
-39
lines changed

runtime/lib/isolate.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,9 +1025,8 @@ class SpawnIsolateTask : public ThreadPool::Task {
10251025
} else if (state_->isolate_group() != nullptr) {
10261026
ASSERT(IsolateGroup::Current() == nullptr);
10271027
const bool kBypassSafepoint = false;
1028-
const bool result = Thread::EnterIsolateGroupAsHelper(
1029-
state_->isolate_group(), Thread::kSpawnTask, kBypassSafepoint);
1030-
ASSERT(result);
1028+
Thread::EnterIsolateGroupAsHelper(state_->isolate_group(),
1029+
Thread::kSpawnTask, kBypassSafepoint);
10311030
state_ = nullptr;
10321031
Thread::ExitIsolateGroupAsHelper(kBypassSafepoint);
10331032
} else {

runtime/vm/compiler/jit/compiler.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,9 +1094,8 @@ BackgroundCompiler::~BackgroundCompiler() {
10941094
}
10951095

10961096
void BackgroundCompiler::Run() {
1097-
bool result = Thread::EnterIsolateGroupAsHelper(
1098-
isolate_group_, Thread::kCompilerTask, /*bypass_safepoint=*/false);
1099-
ASSERT(result);
1097+
Thread::EnterIsolateGroupAsHelper(isolate_group_, Thread::kCompilerTask,
1098+
/*bypass_safepoint=*/false);
11001099
{
11011100
Thread* thread = Thread::Current();
11021101
StackZone stack_zone(thread);

runtime/vm/heap/marker.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,9 +1044,8 @@ class ConcurrentMarkTask : public ThreadPool::Task {
10441044
}
10451045

10461046
virtual void Run() {
1047-
bool result = Thread::EnterIsolateGroupAsHelper(
1048-
isolate_group_, Thread::kMarkerTask, /*bypass_safepoint=*/true);
1049-
ASSERT(result);
1047+
Thread::EnterIsolateGroupAsHelper(isolate_group_, Thread::kMarkerTask,
1048+
/*bypass_safepoint=*/true);
10501049
{
10511050
TIMELINE_FUNCTION_GC_DURATION(Thread::Current(), "ConcurrentMark");
10521051
int64_t start = OS::GetCurrentMonotonicMicros();

runtime/vm/heap/safepoint.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -437,9 +437,8 @@ void SafepointTask::Run() {
437437
return;
438438
}
439439

440-
bool result = Thread::EnterIsolateGroupAsHelper(isolate_group_, kind_,
441-
/*bypass_safepoint=*/true);
442-
ASSERT(result);
440+
Thread::EnterIsolateGroupAsHelper(isolate_group_, kind_,
441+
/*bypass_safepoint=*/true);
443442

444443
RunEnteredIsolateGroup();
445444

runtime/vm/heap/sweeper.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,7 @@ class ConcurrentSweeperTask : public ThreadPool::Task {
176176
}
177177

178178
virtual void Run() {
179-
bool result = Thread::EnterIsolateGroupAsNonMutator(isolate_group_,
180-
Thread::kSweeperTask);
181-
ASSERT(result);
179+
Thread::EnterIsolateGroupAsNonMutator(isolate_group_, Thread::kSweeperTask);
182180
PageSpace* old_space = isolate_group_->heap()->old_space();
183181
{
184182
Thread* thread = Thread::Current();

runtime/vm/isolate.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1804,9 +1804,8 @@ class EnterIsolateGroupScope {
18041804
explicit EnterIsolateGroupScope(IsolateGroup* isolate_group)
18051805
: isolate_group_(isolate_group) {
18061806
ASSERT(IsolateGroup::Current() == nullptr);
1807-
const bool result = Thread::EnterIsolateGroupAsHelper(
1808-
isolate_group_, Thread::kUnknownTask, /*bypass_safepoint=*/false);
1809-
ASSERT(result);
1807+
Thread::EnterIsolateGroupAsHelper(isolate_group_, Thread::kUnknownTask,
1808+
/*bypass_safepoint=*/false);
18101809
}
18111810

18121811
~EnterIsolateGroupScope() {

runtime/vm/thread.cc

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -482,22 +482,19 @@ void Thread::ExitIsolate(bool isolate_shutdown) {
482482
}
483483
}
484484

485-
bool Thread::EnterIsolateGroupAsHelper(IsolateGroup* isolate_group,
485+
void Thread::EnterIsolateGroupAsHelper(IsolateGroup* isolate_group,
486486
TaskKind kind,
487487
bool bypass_safepoint) {
488488
Thread* thread = AddActiveThread(isolate_group, /*isolate=*/nullptr,
489489
/*is_dart_mutator=*/false, bypass_safepoint);
490-
if (thread != nullptr) {
491-
thread->SetupState(kind);
492-
// Even if [bypass_safepoint] is true, a thread may need mutator state (e.g.
493-
// parallel scavenger threads write to the [Thread]s storebuffer)
494-
thread->SetupMutatorState(kind);
495-
ResumeThreadInternal(thread);
490+
RELEASE_ASSERT(thread != nullptr);
491+
thread->SetupState(kind);
492+
// Even if [bypass_safepoint] is true, a thread may need mutator state (e.g.
493+
// parallel scavenger threads write to the [Thread]s storebuffer)
494+
thread->SetupMutatorState(kind);
495+
ResumeThreadInternal(thread);
496496

497-
thread->AssertNonDartMutatorInvariants();
498-
return true;
499-
}
500-
return false;
497+
thread->AssertNonDartMutatorInvariants();
501498
}
502499

503500
void Thread::ExitIsolateGroupAsHelper(bool bypass_safepoint) {
@@ -513,19 +510,16 @@ void Thread::ExitIsolateGroupAsHelper(bool bypass_safepoint) {
513510
bypass_safepoint);
514511
}
515512

516-
bool Thread::EnterIsolateGroupAsNonMutator(IsolateGroup* isolate_group,
513+
void Thread::EnterIsolateGroupAsNonMutator(IsolateGroup* isolate_group,
517514
TaskKind kind) {
518515
Thread* thread =
519516
AddActiveThread(isolate_group, /*isolate=*/nullptr,
520517
/*is_dart_mutator=*/false, /*bypass_safepoint=*/true);
521-
if (thread != nullptr) {
522-
thread->SetupState(kind);
523-
ResumeThreadInternal(thread);
518+
ASSERT(thread != nullptr);
519+
thread->SetupState(kind);
520+
ResumeThreadInternal(thread);
524521

525-
thread->AssertNonMutatorInvariants();
526-
return true;
527-
}
528-
return false;
522+
thread->AssertNonMutatorInvariants();
529523
}
530524

531525
void Thread::ExitIsolateGroupAsNonMutator() {

runtime/vm/thread.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,12 +387,12 @@ class Thread : public ThreadState {
387387
// Makes the current thread exit its isolate.
388388
static void ExitIsolate(bool isolate_shutdown = false);
389389

390-
static bool EnterIsolateGroupAsHelper(IsolateGroup* isolate_group,
390+
static void EnterIsolateGroupAsHelper(IsolateGroup* isolate_group,
391391
TaskKind kind,
392392
bool bypass_safepoint);
393393
static void ExitIsolateGroupAsHelper(bool bypass_safepoint);
394394

395-
static bool EnterIsolateGroupAsNonMutator(IsolateGroup* isolate_group,
395+
static void EnterIsolateGroupAsNonMutator(IsolateGroup* isolate_group,
396396
TaskKind kind);
397397
static void ExitIsolateGroupAsNonMutator();
398398

0 commit comments

Comments
 (0)