- Updated conditional compilation flags throughout the runtime codebase to transition from DART_DYNAMIC_MODULES to DART_BYTECODE_INTERPRETER.
- Adjusted logic in various files including object_graph_copy.cc, object_reload.cc, profiler.cc, and others to ensure compatibility with the new interpreter model.
- Ensured that all references to dynamic modules are replaced with bytecode interpreter checks, maintaining functionality for interpreted code execution.
- Modified stack frame handling and service-related code to align with the new interpreter architecture.
- Updated tests and service implementations to reflect the changes in the runtime environment.
Signed-off-by: Tony <tonylu@tony-cloud.com>
The former contents of the VM isolate are now included into each isolate group. This makes each isolate group's heap independent, and in particular allows each heap to be allocated to a separate pointer cage (not done in this CL).
The duplicated stubs that allowed PC relative calls are removed, since the originals can now be the target of PC relative calls.
The bootstrapping needing to load an AppJIT or AppAOT snapshot is reduced to allocating the oddballs. The code is entirely dropped in the AOT runtime, but the JIT runtime still has it to allow for flags to affect the compilation of the stub code. Further refactoring might be able to remove this for the JIT runtime too, with only gen_snapshot knowing how to bootstrap.
Class serialization no longer distinguishes predefined classes.
The page containing null is marked as never-evacuate. null, false and true must not move because the compiler relies on their low bits having certain patterns for some optimizations. (Previously, the entire VM isolate heap never moved.)
Compaction is disabled for IA32. Due to register pressure, some stub calls must not use a scratch register and embed the address of Code.
The page containing the call-through-safepoint stub is frozen when running with --write-protect-code and the stub is created at runtime (instead of loaded from an AppJIT or AppAOT snapshot). This stub must remain executable even during a safepoint, as a foreign call might during return during a safepoint and only block after the stub directs it to the runtime.
The snapshot symbols are renamed to kDartSnapshotData and kDartSnapshotText. There is no need to distinguish the VM isolate's snapshot, and snaphots are per isolate group not per isolate. Aliases with the old names are added to ease migration.
Some global flags that were automatically set based on the VM isolate's snapshot are now isolate group flags and automatically set by the isolate group's snapshot.
TEST=ci
Change-Id: Iee82016057d609112e9b021d178fc3d4d18b5044
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500621
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
SLSA-Policy-Verified: SLSA Policy Verification Service <devtools-gerritcodereview-exitgate@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Postpone loading of Code and ConstantPool objects during hot reload
until ReloadPhase4CommitFinish in order to avoid creating
constants too early, before class table is switched to use new classes
(otherwise heap would contain a mix of objects with new and old
layouts).
TEST=ci (vm/cc/IsolateReload)
Change-Id: I602987d194c5eb00b2e2a0e070dcd7b50cea0212
Cq-Include-Trybots: luci.dart.try:vm-aot-dyn-linux-debug-x64-try,vm-aot-dyn-linux-product-x64-try,vm-dyn-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/441660
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
In case of compile-time errors kernel service has been serializing kernel AST anyway and has been passing kernel bytes to the VM.
VM does not use those kernel bytes except a few unit tests.
This change removes the unnecessary kernel serialization and freeing
of kernel bytes and cleans up unit tests.
TEST=ci
Change-Id: Ic2464e50f56227bc998df53dd2b594c1abcf9468
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/436360
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
- Don't scavenge when there is no OOM reservation, so refilling the OOM reservation will get the first go once free space is available instead it being consumed by promotions.
- Increase the desired free space in new space needed to continue scavenging. Since new space became paged, 1 KB / 64 pages only guarentees the ability to allocate a 16 byte object.
- Stop issuing stress test reloads after an OOM is encountered.
- Add a HANDLESCOPE to reload to make memory collectible sooner after an error.
TEST=vm/out_of_memory
Bug: https://github.com/dart-lang/sdk/issues/48920
Bug: https://github.com/dart-lang/sdk/issues/53455
Bug: https://github.com/dart-lang/sdk/issues/43642
Change-Id: Ife8bed52e341648a66f78d55bfa60b3944be5d46
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422324
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
When there is an error, we need to return to the VM state before allocating the exception and its message.
TEST=ci (flaky)
Change-Id: I69f37aee1d3f308ab942f1581fc05501507c5346
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/414840
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
With sound null safety both 'is' and 'as' type checks use subtyping
without any extra differences.
Replace Instance::IsAssignableTo with IsInstanceOf (which is now
the same as RuntimeTypeIsSubtypeOf).
Remove Instance::NullIsInstanceOf as it is no longer used.
Replace CompileType::IsAssignableTo and IsInstanceOf with IsSubtypeOf.
TEST=ci
Change-Id: I3a071d114c5f86d990255416b244961d506257a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401861
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Arbitrary ObjectPtr can be a non-heap object (Smi), so methods
ObjectPtr::Is* which test class id should account for Smi.
Changed ObjectPtr::GetClassId to account for Smi similarly to
Object::GetClassId. Added unsafe ObjectPtr::GetClassIdOfHeapObject
which can be used when caller knows the heap nature of the object.
This change also fixes Integer::GetInt64Value which was relying on ObjectPtr::IsSmi.
TEST=vm/cc/Smi, vm/cc/Mint
Change-Id: I1391600e2acedc7b2a8f35c814df113ec9ba8698
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380280
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
When doing fine grained invalidation and reload we might end up losing
`is_implemented` bit and some other class-hierarchy related bits
on a class if we only reload the library where class is declared
but not transitive closure which depends on that library.
Consider:
library a;
class A {}
library b;
class B implements A {}
If we do a change to library `a` which does not impact the outline
then we do not need to reload `b`. This will cause class A to
be replaced with a new `Class` object in the class table - and lead
to us losing various CHA related bits.
This CL update ProgramReloadContext::RebuildDirectSubclasses to
handle this correctly, we also rename it to
RestoreClassHierarchyInvariants and make it share implementation
with ClassFinalizer (which contained almost identical function).
Fixes https://github.com/flutter/flutter/issues/151032
TEST=vm/cc/IsolateReload_IsImplementedBit
Change-Id: Ie620592befecb89897d6e8d46175ef07348bf11d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/374040
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Since null safety, all static fields with initializers are implicitly
late. This change cleans up transition_sentinel which was used in
the detection of cyclic initialization of legacy static fields.
TEST=ci
Change-Id: I6a990dc8ba030f5bd40eb0b86706cbfb0f725e33
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373520
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
When uploading CLs, the presubmit checks verify that the lines in the
diff are formatted correctly according to `git cl format runtime`.
However, when `buildtools/<os>-<arch>/clang/bin/clang-format` is
updated, it does not force reformatting of files that would be
reformatted.
This leads to two issues:
* Inconsistent style within the code base and within a single file.
* Spurious reformatting in CLs when (1) clang-format is used on the
whole file, or (2) the diff lines overlap.
`clang-format` doesn't change that frequently, so in general this is
not a large issue, but I've seen a bit too many "spurious formatting,
please revert" comments on CLs recently.
This CL formats the runtime to be in line with the current pinned
`clang-format`:
```
$ find runtime/ -iname *.h -o -iname *.cc | xargs buildtools/mac-arm64/clang/bin/clang-format -i
```
`git cl format` (which only formats changed lines, and does so with
`clang-format`) seems to not agree with itself, or clang-format, or
cpplint in a handful of places. This CL adds `// clang-format off`
for these. (See previous patchsets for the specific instances.)
TEST=A variety of bots including GCC, MacOS and Windows.
Change-Id: I470892e898971899fda14bb3b8f2c8efefd67686
Cq-Include-Trybots: luci.dart.try:vm-gcc-linux-try,vm-ffi-qemu-linux-release-riscv64-try,vm-ffi-qemu-linux-release-arm-try,vm-aot-win-debug-x64-try,vm-win-debug-x64c-try,vm-mac-debug-x64-try,vm-mac-debug-arm64-try,vm-aot-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/362780
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Enum forwarding requires evaluating and canonicalizing the new enum instances. If these in turn refer to other instances of classes with shape changes, canonicalization may try to compare instances of the old and new shapes and dereference past the end of an object. By waiting for instance morphing to complete, canonicalization can only see instances with the new shape.
Also, don't attempt to forward Enum.values. When an enum member is added or removed, this has the same merging problem as 40442.
Cf. bad074cc49.
TEST=ci
Bug: https://github.com/flutter/flutter/issues/129177
Bug: https://github.com/dart-lang/sdk/issues/53039
Change-Id: Ib7a54db30c4d16a6ae6e1acd595dc7482134165b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/316527
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Enum forwarding requires evaluating and canonicalizing the new enum instances. If these in turn refer to other instances of classes with shape changes, canonicalization may try to compare instances of the old and new shapes and dereference past the end of an object. By waiting for instance morphing to complete, canonicalization can only see instances with the new shape.
Also, don't attempt to forward Enum.values. When an enum member is added or removed, this has the same merging problem as 40442.
Cf. bad074cc49.
TEST=ci
Bug: https://github.com/flutter/flutter/issues/129177
Change-Id: I19d0508059bade8496000eeea257bb0730f11d17
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/316041
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
a) Remove 2 fields in our object representation:
* PatchClass::library_data_
* Library::kernel_data_
=> This saves O(#libraries + #patch-classes) which can amount up to
10+ MB for big apps, as we save not just the slots but the
[ExternalTypedData] objects they used to reference.
Instead we'll compute the KernelLibraryData when we need it by making a
[TypedDataView] of sub parts of the component.
b) We make a kernel binary be represented by a single [ExternalTypedData].
Whenever we need a sub-part of a kernel binary (e.g. one for each
component for concatinated kernels, or one for a library inside a
component) we use proper [TypedDataView]s for that.
c) As we sometimes need to create a view of only a particular library
within a component we need to find start/end of a library based on
index.
=> We store the library index instead of the library offset on
Library/PatchClass.
=> We can easily derive the start/end of a library from it's index by
looking at the kernel component encoding.
d) We make the [Reader] object work purely based on a pointer - instead
of making it have if/else when reading bytes (either from pointer or
from a view).
e) We make the [KernelProgramInfo] store the kernel_component and
various TD views into it.
TEST=ci
Change-Id: Ibe160881ff48635e834c3d647a977a144b5d0565
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313561
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Previously, inliner was able to inline closure calls only when closure
was allocated in the current function (or in a function which was
inlined so far). This is not true for local functions declared in outer
functions (they are allocated at declaration site).
Now, when calling local functions, target of closure call is always
known regardless of where the function is declared, so calls to local
functions can be inlined.
TEST=runtime/tests/vm/dart/inline_local_functions_il_test.dart
Fixes https://github.com/dart-lang/sdk/issues/52695
Fixes https://github.com/dart-lang/sdk/issues/15558
Change-Id: I0e51dbaf63a6d4427bff366b40414ca5fac3d418
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311465
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This field is now only used when reloading to check assignability
during `ProgramReloadContext::ReloadPhase4CommitFinish()` (via
`PostCommit()` -> `InvalidateWorld()` -> `InvalidateFields`).
However, the method `DeoptimizeDependentCode()` is called near the
start of the actual reloading process in `Reload` prior to the
commit/rollback phases. This calls `DeoptimizeTypeTestingStubs()`, which
walks the heap and resets the contents of all `SubtypeTestCaches`
(`STC`s).
That means that the `STC` information isn't actually kept between
different reloads, so all this does is cause a number of `STC` objects
to be allocated during reload whose backing arrays are not used
post-reload for anything and so are unnecessary garbage.
That, and the creation of the `STC`s during `InvalidateFields` creates
all-input `STC`s, as they don't just check assignability to a given field type but also for checking elements of `RecordTypes`. That means
the per-field `STC`s contain way more information needed to check for
valid assignments to the corresponding field type, even if the `STC`
was used between reloads.
This CL removes the unnecessary field and changes `InvalidateFields`
so that the `STC`s used during reload are instead lazily allocated once
per `FieldInvalidator` object and now both the `STC` and its backing
array are now garbage post-field invalidation.
TEST=reload ci
Change-Id: I3ffa199551ba69f0afd4eba6303c6ff73d7473a3
Cq-Include-Trybots: luci.dart.try:vm-reload-linux-debug-x64-try,vm-reload-linux-release-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-reload-rollback-linux-release-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-product-x64-try,vm-aot-linux-release-x64-try,vm-linux-release-x64-try,vm-linux-debug-x64-try,vm-aot-dwarf-linux-product-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311220
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Previously, SlowTypeTestStub would use the incoming instance and
destination type to determine which `SubtypeNTestCacheStub` to call,
where `N` is the number of inputs checked for each entry. The code
for generating `InstanceOf` instructions performs similar work,
inlining some checks and then falling back to checking and updating
an appropriate `SubtypeNTestCacheStub` if the inlined checks fail.
Meanwhile, the runtime would use all available information to
create a new entry when needed, depending on the `SubtypeNTestCacheStub`
called at runtime to ignore the parts of the entry it didn't need.
To illustrate a situation where the runtime might use more
information to create an entry than needed by the stub, consider
the following class header:
```
class C<T> extends D
```
If no subtype of `D` requires knowing the specific type arguments for
an instance to determine whether that instance is an instance of `D`,
then the compiler can generate a `Subtype1TestCacheStub` call, where
only the class id of the instance is checked.
However, in the runtime, when updating the cache, the runtime will
create an entry that includes the specific type arguments used to
instantiate `C` when the initial cache miss occurred, as the runtime
does not know how many inputs the code that checks that SubtypeTestCache
uses. If the cache has grown to the point that it is hash-based, that
means the runtime uses the instance type arguments of the first cache
miss to determine the correct place to create a new entry.
When the stub only performs a linear search over the cache contents,
this works, as every entry is checked until a matching entry is found,
and thus the stub can ignore unneeded information. However, if a cache
is hash-based, this doesn't work: if the first miss was an instance
of `C<int>`, then the runtime includes all that information in a new
entry. This means the stub has to calculate the instance type
arguments, which it did not have to do before, and if a subsequent
check uses different instance type arguments (e.g., an instance of
`C<String>`), the result may still be a cache miss, and the runtime
adds another entry which has the same information as the previous
entry as far as the stub was originally concerned.
Thus, to limit itself to considering only the inputs used by the
appropriate `SubtypeNTestCacheStub`, the runtime needs to be told how
many inputs are used. This CL adds an additional field to
`SubtypeTestCache`s which contains the number of used inputs, and
changes the places where `SubtypeTestCache`s are allocated in the
flow graph compiler and the `DRT_TypeCheck` runtime entry to pass the
appropriate number of inputs. To ensure that the runtime and stubs
agree on how many inputs should be used, the `SlowTypeTestStub` now
uses the value of this field to determine which `SubtypeNTestCacheStub`
to call.
TEST=vm/cc/STC_{Linear,Hash}Lookup
Bug: https://github.com/dart-lang/sdk/issues/48345
Change-Id: Ie16f2d12c4c5eae856b668f28178818203662d1c
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-simriscv64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-product-x64-try,vm-aot-linux-release-simarm64-try,vm-aot-linux-release-simarm_x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-aot-dwarf-linux-product-x64-try,vm-linux-release-ia32-try,vm-linux-debug-x64c-try,vm-linux-debug-x64-try,vm-linux-debug-simriscv64-try,vm-linux-release-simarm64-try,vm-linux-release-simarm-try,vm-mac-release-arm64-try,vm-mac-release-x64-try,vm-tsan-linux-release-x64-try,vm-reload-rollback-linux-release-x64-try,vm-reload-linux-release-x64-try,vm-ffi-qemu-linux-release-arm-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310180
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
We use specialized type testing stubs in `as` checks to quickly answer
whether the type of an object is assignable to a given type. However,
perfect specialization of the type testing stub is not possible in all
cases. Thus, we use SubtypeTestCaches (STCs) to cache the result of
checking assignability in the runtime to avoid repeated runtime calls
for the same check.
Previously, the STC was linear and capped at 100 entries. This cap was
for two reasons:
* STC are created per `as` check, not per checked type, so there could
be many STCs created for a given type if assignability to that type
is checked many times throughout the code.
* STC were strictly linear, and thus eventually the cost of
traversing the cache outweighs the cost of returning a false
negative that requires going to the runtime and doing the check there.
This CL provides a partial solution to the second reason by performing
similar work to that done for instantiation caches in TypeArguments
(4f925105), where the STC is linear up to a certain threshold, and then
converted to a hash-based STC. Like that CL, only the runtime currently
handles the lookup of cached results in hash-based STCs, with the
changes needed for the `SubtypeNTestCacheStub`s coming in a followup CL.
Since the stubs have not yet been adjusted, we keep the maximum size of
linear caches the same at 100, and allow hash-based STCs to grow to a
maximum size of 1000 since the STCs are still created per `as` check.
This means once the STC has grown over 100 entries, all STC checks
require a runtime call, but for checks cached in the STC, the answer is
found and provided without either doing the slower full runtime check
for assignability and without an attempt to respecialize the type
testing stub when applicable.
Our benchmarks generally see at most a ~2-5% performance regression for
cases where the STC remains linear (except for on the Raspberry Pi 4, where the hit ranges from ~25%-40%), but a performance improvement of
~500-1300% for cases where the STC is converted to a hash table.
TEST=vm/cc/TTS_STC_{Some,Many}Asserts
Change-Id: Ie8629708b5c12fa8464359a2f77c6da56ba46c0c
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-simriscv64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-product-x64-try,vm-aot-linux-release-simarm64-try,vm-aot-linux-release-simarm_x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-aot-dwarf-linux-product-x64-try,vm-linux-release-ia32-try,vm-linux-debug-x64c-try,vm-linux-debug-x64-try,vm-linux-debug-simriscv64-try,vm-linux-release-simarm64-try,vm-linux-release-simarm-try,vm-mac-release-arm64-try,vm-mac-release-x64-try,vm-tsan-linux-release-x64-try,vm-reload-rollback-linux-release-x64-try,vm-reload-linux-release-x64-try,vm-ffi-qemu-linux-release-arm-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284682
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
The [ReloadOperationScope] has [StackResource]s as fields and is itself
a [StackResource] which is problemantic if unwinding happens manually.
So we'll make it a macro that expands to the 3 fields instead.
TEST=ci
Change-Id: I3fb7bec7ca87193c83ec34908f9a43c5db005900
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302201
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The current hot-reload implementation [0] will perform a reload by
first sending OOB messages to all isolates and waiting until those OOB
messages are being handled. The handler of the OOB message will block
the thread (and unschedule isolate) and notify the thread performing
reload it's ready.
This requires that all isolates within a group can actually run & block.
This is the case for the VM implementation of isolates (as they are
run an unlimited size thread pool).
Though flutter seems to multiplex several engine isolates on the same OS
thread. Reloading can then result in one engine isolate performing
reload waiting for another to act on the OOB message (which it will not
do as it's multiplexed on the same thread as the former).
Now that we have a more flexible safepointing mechanism (introduced in
[1]) we can utilize for hot reloading by introducing a new "reloading"
safepoint level.
Reload safepoints
-----------------------
We introduce a new safepoint level (SafepointLevel::kGCAndDeoptAndReload).
Being at a "reload safepoint" implies being at a "deopt safepoint"
which implies being at a "gc safepoint".
Code has to explicitly opt-into making safepoint checks participate /
check into "reload safepoints" using [ReloadParticipationScope]. We do
that at certain well-defined places where reload is possible (e.g. event
loop boundaries, descheduling of isolates, OOM message processing, ...).
While running under [NoReloadScope] we disable checking into "reload
safepoints".
Initiator of hot-reload
-----------------------
When a mutator initiates a reload operation (e.g. as part of a
`ReloadSources` `vm-service` API call) it will use a
[ReloadSafepointOperationScope] to get all other mutators to a
safepoint.
For mutators that aren't already at a "reload safepoint", we'll
notify them via an OOB message (instead of scheduling kVMInterrupt).
While waiting for all mutators to check into a "reload safepoint", the
thread is itself at a safepoint (as other mutators may perform lower
level safepoint operations - e.g. GC, Deopt, ...)
Once all mutators are at a "reload safepoint" the thread will take
ownership of all safepoint levels.
Other mutators
-----------------------
Mutators can be at a "reload safepoint" already (e.g. isolate is not
scheduled). If they try to exit safepoint they will block until the
reload operation is finished.
Mutators that are not at a "reload safepoint" (e.g. executing Dart or VM
code) will be sent an OOB message indicating it should check into a
"reload safepoint". We assume mutators make progress until they can
process OOB message.
Mutators may run under a [NoReloadScope] when handling the OOM message.
In that case they will not check into the "reload safepoint" and simply
ignore the message. To ensure the thread will eventually check-in,
we'll make the destructor of [~NoReloadScope] check & send itself a new OOB
message indicating reload should happen. Eventually getting the mutator
to process the OOM message (which is a well-defined place where we can
check into the reload safepoint).
Non-isolate mutators such as the background compiler do not react to OOB
messages. This means that either those mutators have to be stopped (e.g.
bg compiler) before initiating a reload safepoint operation, the
threads have to explicitly opt-into participating in reload safepoints
or the threads have to deschedule themselves eventually.
Misc
----
Owning a reload safepoint operation implies also owning the deopt &
gc safepoint operation. Yet some code would like to ensure it actually
runs under a [DeoptSafepointOperatoinScope]/[GCSafepointOperationScope].
=> The `Thread::OwnsGCSafepoint()` handles that.
While performing hot-reload we may exercise common code (e.g. kernel
loader, ...) that acquires safepoint locks. Normally it's disallows to
acquire safepoint locks while holding a safepoint operation (since
mutators may be stopped at places where they hold locks, creating
deadlock scenarios).
=> We explicitly opt code into participating in reload safepointing
requests. Those well-defined places aren't holding safepoint locks.
=> The `Thread::CanAcquireSafepointLocks()` will return `true` despite
owning a reload operation. (But if one also holds deopt/gc safepoint
operation it will return false)
Example where this matters: As part of hot-reload, we load kernel which
may create new symbols. The symbol creation code may acquire the symbol
lock and `InsertNewOrGet()` a symbol. This is safe as other mutators
don't hold the symbol lock at reload safepoints. The same cannot be said
for Deopt/GC safepoint operations - as they can interrupt code at many
more places where there's no guarantee that no locks are held.
[0] https://dart-review.googlesource.com/c/sdk/+/187461
[1] https://dart-review.googlesource.com/c/sdk/+/196927
Issue https://github.com/flutter/flutter/issues/124546
TEST=Newly added Reload_* tests.
Change-Id: I6842d7d2b284d043cc047fd702b7c5c7dd1fa3c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296183
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
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>
This is a reland of commit 87810072db
Original change's description:
> [vm] Account for changes in the implicit getters due to load guards
>
> Hot reload may mark certain fields as 'needs_load_guard', meaning
> types of their values should be checked on access, in the implicit
> getters.
>
> Unoptimized code for implicit getters of such fields should be
> forcefully recompiled during hot reload in order to update their
> code. This ensures that optimized code in future can deoptimize
> into the updated unoptimized code of implicit getters.
TEST=vm/cc/IsolateReload_ImplicitGetterWithLoadGuard
TEST=corelib/string_fromcharcodes_test, lib/convert/json_utf8_chunk_test
and vm/dart/string_equals_test on vm-reload-linux-{debug,release}-x64
configurations.
Fixes https://github.com/dart-lang/sdk/issues/51215
Fixes https://github.com/flutter/flutter/issues/103268
Change-Id: I22eb0f7addce387e7773e45c170c3f9050003083
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/282807
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Tony