Skip to content

Commit

Permalink
[vm] Avoid repeatedly re-setting stack limits on each dart invocation
Browse files Browse the repository at this point in the history
Currently every invocation of a dart function will set and later on
reset the stack limits. Doing so requires acquiring locks.

Similarly because we have async ffi callbacks (another way to invoke
dart code) the logic was duplicated there.

Though the stack limit never changes for a given [OSThread]. Isolates
can run on different [OSThread]s throughtout its lifetime. But an
isolate always has to be entered on a native thread before it can
execute dart code.

=> We initialize the stack limit when we scheduling an isolate on a
thread and re-set it when unscheduling it.

=> That centralizes the place to one where we have to deal with stack
limits and avoids repeated acquiring of locks on each embedder dart
function invocation.

TEST=ci

Change-Id: Ia59ba8f92b93c58a990010ec75dfcd879aea2c43
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311960
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
  • Loading branch information
mkustermann authored and Commit Queue committed Jun 29, 2023
1 parent ac77af1 commit 45038a2
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 24 deletions.
13 changes: 0 additions & 13 deletions runtime/vm/dart_entry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,6 @@ class DartEntryScope : public TransitionToGenerated {
saved_long_jump_base_ = thread->long_jump_base();
thread->set_long_jump_base(nullptr);

// Setup the stack limit checked by generated Dart code. This is repeated at
// each Dart entry because a given Thread may move between different
// OSThreads.
saved_stack_limit_ = thread->saved_stack_limit();
#if defined(USING_SIMULATOR)
thread->SetStackLimit(Simulator::Current()->overflow_stack_limit());
#else
thread->SetStackLimit(OSThread::Current()->overflow_stack_limit());
#endif

#if defined(USING_SAFE_STACK)
// Remember the safestack pointer at entry so it can be restored in
// Exceptions::JumpToFrame when a Dart exception jumps over C++ frames.
Expand All @@ -68,15 +58,12 @@ class DartEntryScope : public TransitionToGenerated {
thread()->set_saved_safestack_limit(saved_safestack_limit_);
#endif

thread()->SetStackLimit(saved_stack_limit_);

ASSERT(thread()->long_jump_base() == nullptr);
thread()->set_long_jump_base(saved_long_jump_base_);
}

private:
LongJumpScope* saved_long_jump_base_;
uword saved_stack_limit_ = 0;
#if defined(USING_SAFE_STACK)
uword saved_safestack_limit_ = 0;
#endif
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -1740,7 +1740,7 @@ class StartIsolateScope {
ASSERT(Isolate::Current() == nullptr);
Thread::EnterIsolate(new_isolate_);
// Ensure this is not a nested 'isolate enter' with prior state.
ASSERT(Thread::Current()->saved_stack_limit() == 0);
ASSERT(Thread::Current()->top_exit_frame_info() == 0);
}
}

Expand All @@ -1753,7 +1753,7 @@ class StartIsolateScope {
if (saved_isolate_ != new_isolate_) {
ASSERT(saved_isolate_ == nullptr);
// ASSERT that we have bottomed out of all Dart invocations.
ASSERT(Thread::Current()->saved_stack_limit() == 0);
ASSERT(Thread::Current()->top_exit_frame_info() == 0);
Thread::ExitIsolate();
}
}
Expand Down
2 changes: 2 additions & 0 deletions runtime/vm/os_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ class BaseThread {
// Low-level operations on OS platform threads.
class OSThread : public BaseThread {
public:
static const uword kInvalidStackLimit = ~static_cast<uword>(0);

// The constructor of OSThread is never called directly, instead we call
// this factory style method 'CreateOSThread' to create OSThread structures.
// The method can return a nullptr if the Dart VM is in shutdown mode.
Expand Down
1 change: 0 additions & 1 deletion runtime/vm/runtime_entry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4026,7 +4026,6 @@ extern "C" Thread* DLRT_GetFfiCallbackMetadata(
target_isolate->group()->EnterTemporaryIsolate();
Thread* const temp_thread = Thread::Current();
ASSERT(temp_thread != nullptr);
temp_thread->SetStackLimit(OSThread::Current()->overflow_stack_limit());
temp_thread->set_unboxed_int64_runtime_arg(metadata2.send_port());
temp_thread->set_unboxed_int64_runtime_second_arg(
reinterpret_cast<intptr_t>(current_isolate));
Expand Down
31 changes: 24 additions & 7 deletions runtime/vm/thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ void Thread::AssertEmptyThreadInvariants() {
ASSERT(deferred_marking_stack_block_ == nullptr);
ASSERT(!is_unwind_in_progress_);

ASSERT(saved_stack_limit_ == 0);
ASSERT(saved_stack_limit_ == OSThread::kInvalidStackLimit);
ASSERT(stack_limit_.load() == 0);
ASSERT(safepoint_state_ == 0);

Expand Down Expand Up @@ -401,7 +401,7 @@ void Thread::EnterIsolate(Isolate* isolate) {
}

isolate->scheduled_mutator_thread_ = thread;
ResumeThreadInternal(thread);
ResumeDartMutatorThreadInternal(thread);
}

static bool ShouldSuspend(bool isolate_shutdown, Thread* thread) {
Expand Down Expand Up @@ -450,7 +450,7 @@ void Thread::ExitIsolate(bool isolate_shutdown) {
if (ShouldSuspend(isolate_shutdown, thread)) {
const auto tag =
isolate->is_runnable() ? VMTag::kIdleTagId : VMTag::kLoadWaitTagId;
SuspendThreadInternal(thread, tag);
SuspendDartMutatorThreadInternal(thread, tag);
{
// Descheduled isolates are reloadable (if nothing else prevents it).
RawReloadParticipationScope enable_reload(thread);
Expand All @@ -461,7 +461,7 @@ void Thread::ExitIsolate(bool isolate_shutdown) {
thread->ResetDartMutatorState(isolate);
thread->ResetMutatorState();
thread->ResetState();
SuspendThreadInternal(thread, VMTag::kInvalidTagId);
SuspendDartMutatorThreadInternal(thread, VMTag::kInvalidTagId);
FreeActiveThread(thread, /*bypass_safepoint=*/false);
}

Expand Down Expand Up @@ -528,6 +528,24 @@ void Thread::ExitIsolateGroupAsNonMutator() {
FreeActiveThread(thread, /*bypass_safepoint=*/true);
}

void Thread::ResumeDartMutatorThreadInternal(Thread* thread) {
ResumeThreadInternal(thread);
if (Dart::vm_isolate() != nullptr &&
thread->isolate() != Dart::vm_isolate()) {
#if defined(USING_SIMULATOR)
thread->SetStackLimit(Simulator::Current()->overflow_stack_limit());
#else
thread->SetStackLimit(OSThread::Current()->overflow_stack_limit());
#endif
}
}

void Thread::SuspendDartMutatorThreadInternal(Thread* thread,
VMTag::VMTagId tag) {
thread->ClearStackLimit();
SuspendThreadInternal(thread, tag);
}

void Thread::ResumeThreadInternal(Thread* thread) {
ASSERT(!thread->IsAtSafepoint());
ASSERT(thread->isolate_group() != nullptr);
Expand Down Expand Up @@ -599,7 +617,7 @@ Thread* Thread::AddActiveThread(IsolateGroup* group,
thread->runtime_call_deopt_ability_ = RuntimeCallDeoptAbility::kCanLazyDeopt;
ASSERT(!thread->IsAtSafepoint());

ASSERT(thread->saved_stack_limit_ == 0);
ASSERT(thread->saved_stack_limit_ == OSThread::kInvalidStackLimit);
return thread;
}

Expand Down Expand Up @@ -676,7 +694,7 @@ void Thread::SetStackLimit(uword limit) {
}

void Thread::ClearStackLimit() {
SetStackLimit(~static_cast<uword>(0));
SetStackLimit(OSThread::kInvalidStackLimit);
}

static bool IsInterruptLimit(uword limit) {
Expand Down Expand Up @@ -1364,7 +1382,6 @@ void Thread::ResetDartMutatorState(Isolate* isolate) {
ASSERT(execution_state() == Thread::kThreadInVM);

isolate->mutator_thread_ = nullptr;
saved_stack_limit_ = 0; // Isolate shutdown may set this to 0xff..
is_unwind_in_progress_ = false;

field_table_values_ = nullptr;
Expand Down
6 changes: 5 additions & 1 deletion runtime/vm/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,7 @@ class Thread : public ThreadState {
Isolate* isolate_ = nullptr;
IsolateGroup* isolate_group_ = nullptr;

uword saved_stack_limit_ = 0;
uword saved_stack_limit_ = OSThread::kInvalidStackLimit;
// The mutator uses this to indicate it wants to OSR (by
// setting [Thread::kOsrRequest]) before going to runtime which will see this
// bit.
Expand Down Expand Up @@ -1413,6 +1413,10 @@ class Thread : public ThreadState {
void SetupDartMutatorStateDependingOnSnapshot(IsolateGroup* group);
void ResetDartMutatorState(Isolate* isolate);

static void SuspendDartMutatorThreadInternal(Thread* thread,
VMTag::VMTagId tag);
static void ResumeDartMutatorThreadInternal(Thread* thread);

static void SuspendThreadInternal(Thread* thread, VMTag::VMTagId tag);
static void ResumeThreadInternal(Thread* thread);

Expand Down

0 comments on commit 45038a2

Please sign in to comment.