commit 2e466f66dbbb1d152e3205032954659085a0f3d5
author Martin Kustermann <kustermann@google.com> Fri Apr 21 08:06:49 2023 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Apr 21 08:06:49 2023 +0000
tree badafec443ddd7411f8d685c1870229b5122f089
parent b985fc961a873ac79d170be18756febfd2526614

[vm] Refactor thread scheduling code to better handle exits with active stack

An embedder (or the VM) can exit an isolate via `Thread::ExitIsolate()`
at a point where there's still active state (e.g. dart frames).

Because of this the VM has so far conservatively retained the [Thread]
object of dart mutators throughout the isolate's lifetime. After which
is was manually `delete`ed. We'd never re-use those [Thread] objects (we
do re-use [Thread] objects of non-dart-mutator threads).

When exiting via `Thread::ExitIsolate()` with active state, the mutator
was assumed to be at-safepoint at all levels. It was removed from the
thread registry's active threads. This also means that when e.g. GC runs
it can't use the thread registry to find all active threads it may
need to scan, instead it uses [Isolate::mutator_thread_] of all isolates.

This causes a variety of subtle issues, but the main one that motivated
this change is the following:

If a thread obtains a safepoint operation it means all other mutators
are parked. The thread owning the safepoint can do whatever it likes.
When introducing reload operation safepoints, a thread may want to

    ReloadSafepointOperation reload(thread);
    ...

    // Compile sources.
    {
      TransitionVMToNative transition(thread);

      // Will temporarily exit & re-enter current isolate.
      response_port = Dart_NewNativePort();

      Dart_PostCObject(kernel_isolate_port, ...);

      // Wait on [response_port] for response.
    }

This will cause the reloading thread to own the reload safepoint
operation but still transition states and even exit/re-enter the
isolate. Though this is currently not possible in the way enter/exit is
implemented.

So we'll refactor this fragile code in the following way:

* Move thread enter/exit logic entirely to the [Thread] object.

* Keep used threads in the thread registry's active list.

  => This allows us to keep various state on the [Thread] and thereby
  avoids clearing it when suspending & re-initialing it when resuming

  => It makes nested `Thread::ExitIsolate()` faster as we mainly have
  to enter safepoint (avoid acquiring threads lock, avoid releasing
  storebuffers, ...)

  => It makes nested `Thread::EnterIsolate()` faster as we mainly have
  to leave the safepoint (avoid acquiring threads lock, avoid acquiring
  storebuffers, ...).

  => A mutator can now own a safepoint operation (e.g. reload safepoint
  operation) and still `ExitSafepoint()` / `EnterSafepoint()` safely -
  as those are based on the normal `EnterSafepoint()` and
  `LeaveSafepoint()` APIs.

* We separate

   - Suspend & Resume of a dart mutator (possibly with active stack)
   - Setup & Reset of state only relevant for dart mutators
   - Setup & Reset of state relevant for any mutator

* We unify how the [Thread] objects are freed between dart mutator and
  non-dart mutators: [Thread] objects without state can be given back to
  the [ThreadRegistry] and re-used (instead of being deleted in
  `Isolate::~Isolate`)

* We have capability to free [Thread] objects if a dart mutator has an
  empty stack & re-use for another isolate of the same group.
  (In future we may have N Thread objects for N cores and the threads
   would even maintain their TLABs when switching between isolates)

* Since we allow reusing of [Thread] objects also for dart mutators now,
  we have extensive asserts to ensure they are "clean" when they get
  into the free list and come out "clean" again.

TEST=ci

Change-Id: Id85e8e484efd98d28e323b33795716420e619986
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296585
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>