This is the initial implementation of hot reload with multi-isolate
groups.
Implementation:
As before, when a service API call triggers a reload it will be routed
as an OOB message to a specific isolate (**). As opposed to before, that
isolate has now to coordinate with all other isolates, ensuring that it
"owns" the reload and all other isolates are waiting in a state that
allows reload.
This is implemented as a [ReloadOperationScope] which first participates
in other reloads (if there are any) and then owns the reload. It will
send a new kind of service message to all other registered isolates. All
of them have to check in before reload can proceed. If a new isolate
is about to join the group, it will participate when registering the
isolate. If an old isolate wants to die, it will participate when
unregistering the isolate.
This means that in addition to the existing StackOverFlow checks that
can process OOB messages and therefore reload, we'll have isolate
registration and unregistration as well as a new
Isolate::kCheckForReload OOB message handler where an isolate can
participate in a reload.
We consider the isolate group to be reloadable if the main isolate has
loaded the program and set the root library. Helper isolates don't need
to load any more kernel code and only initialize core libraries, so it's
fine to reload them during this time.
(**) The reason we continue to send reload service API calls to any
isolate in an isolate group is that re-loading might involve calling out
to the embedder's tag handler. Doing so currently requires an active
isolate.
If we allowed a subset of dart_api.h (the subset needed by the tag
handler) to be used only with an active IsolateGroup instead of an
active Isolate we could remove this requirement.
Edge cases:
There's various edge cases to consider: The main edge case is, we currently
maintain an upper limit to the number of isolates executing in parallel
(to ensure each can have big enough chunk of new space, i.e. TLAB).
If there are more isolates with active work they are waiting until one
of the exiting ones "yields". To ensure progress, if any such actively
running isolate gets a request to participate in a reload, it will mark
its own thread as "blocked" and therefore "yields", so another isolate
can make progress until all isolates are participating and the reload
can start.
Marking an isolate as "blocked" happens by exiting that isolate. It will
free up it's TLAB, decrease active mutator count and (if running on VM's
thread pool) also temporarily increase the thread pool size.
The side-effect of this is that it will use one pthread per isolate
during reload. In the future we can extend this first implementation, by
specially handling isolates that don't have a message handler running.
Doing so would require careful consideration to avoid races.
Testing:
In order to test this we use a small helper framework for reload tests.
The helper framework will, similar to real world reload e.g. in flutter,
will spawn a subprocess. It will use the service API to trigger reloads
in this subproces.
To synchronize between the reload driver and the application being
reloaded it allows watching for events to be printed to stdout/stderr.
The reload test itself can be written - similar to multitests - with
annotations such as `// @include-in-relload-0` in them. The testing
framework will then generate multiple application versions that all get
compiled to kernel.
For simplicity we generate the kernel using the standalone VM with
`--snapshot-kind=kernel` and avoid using the incremental compiler.
There are 4 different tests exercising different aspects of
multi-isolate reload:
vm/dart_2/isolates/reload_active_stack_test:
Performs a reload while a fixed number of isolates have an active
stack, thereby ensuring e.g. that all frames of all isolate mutator
stacks get deoptimized, ...
vm/dart_2/isolates/reload_no_active_stack_test:
Similar to the test above, but instead of having an active stack the
isolates can yield to the event loop, possibly be even descheduled
vm/dart_2/isolates/reload_many_isolates_test:
Similar to the test above, but this test uses many more isolates.
vm/dart_2/isolates/reload_many_isolates_live_and_die_test:
Performs a reload where isolates get spawned and die all the time.
There are always P isolates alive at any given point in time, each
of them spawns children when their parent has died.
Performing a reload catches isolates as various stages of their
lifecycle and can therefore cover a lot of corner cases.
TEST=vm/dart_2/isolates/reload_*_test.dart
Issue https://github.com/dart-lang/sdk/issues/36097
Change-Id: I97039b4084de040b7f2e22f5832a40d57ba398d5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/187461
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Whether a given isolate allows reloading is really a property of the
thread not the isolate (where it was before) or isolate group (where
it was moved to), so this CL moves it to Thread.
As the thread's stack is going down and up, NoReloadScope's might be
entered or exited, just as with our many other scopes, such as
NoOOBMessageScope/...
Longer term we might need to find a better mechanism to deal with
different OOB messages. For example: Reload as well as OOB messages
are disabled during static field initialization. Though that means *all*
OOB messages cannot reach the isolate during initialization of a static
field (which can do arbitrary amount of work).
This is part of making hot-reload work with isolate groups.
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=Refactoring of existing code.
Change-Id: I9e80c6d4a184c54c0373fef13af3264cc3f27ece
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/187002
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
If the intrinsic on int.op fails, then the intrinsic on _opFromInteger will also fail, since both handle the same range of values.
Add intrinsic on int.% as previously only _moduloFromInteger was intrinsified. All the other ops already intrinsified the public method.
Remove _bitAndFromSmi; this was once useful for handling BigInt & Smi, but this case no longer exists.
TEST=ci
Change-Id: Ie33b75126e56110db3caabb9bcb6e9198a3158bf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/187160
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This is a reland of b6dc4dad4d
TEST=ci
Original change's description:
> [vm/aot] Avoid using most Code objects in stack traces with --dwarf-stack-traces
>
> The following changes are done in preparation for the removal of Code
> objects in AOT with --dwarf-stack-traces:
>
> * Stack trace objects are extended to hold uword PCs (which may not
> fit into Smi range).
>
> * Scanning stack frames in GC (StackFrame::VisitObjectPointers)
> now avoids using Code objects.
> In order to find CompressedStackMaps it now calls
> ReversePc::FindCompressedStackMaps.
>
> * Singleton Code object (StubCode::UnknownDartCode()) is prepared as
> a replacement for Code objects in stack traces. It has
> PayloadStart() == 0 and Size() == kUwordMax so it includes
> arbitrary PCs.
>
> * In --dwarf-stack-traces mode, most Code objects obtained from stack
> frames are replaced with StubCode::UnknownDartCode().
> This simulates future behavior of ReversePc::Lookup when Code objects
> will be removed.
>
> Issue: https://github.com/dart-lang/sdk/issues/44852
> Change-Id: I7cec7b8b9396c9cfeca3c256a412ba4e82a7e0c4
> TEST=ci
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/182720
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Tess Strickland <sstrickl@google.com>
> Reviewed-by: Martin Kustermann <kustermann@google.com>
Change-Id: Ia2fc4672a085cd963b7fc103369851df3603590c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/186202
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
This reverts commit b6dc4dad4d.
Reason for revert: broke package:vm_snapshot_analysis in Flutter
(https://github.com/flutter/flutter/issues/76313).
Original change's description:
> [vm/aot] Avoid using most Code objects in stack traces with --dwarf-stack-traces
>
> The following changes are done in preparation for the removal of Code
> objects in AOT with --dwarf-stack-traces:
>
> * Stack trace objects are extended to hold uword PCs (which may not
> fit into Smi range).
>
> * Scanning stack frames in GC (StackFrame::VisitObjectPointers)
> now avoids using Code objects.
> In order to find CompressedStackMaps it now calls
> ReversePc::FindCompressedStackMaps.
>
> * Singleton Code object (StubCode::UnknownDartCode()) is prepared as
> a replacement for Code objects in stack traces. It has
> PayloadStart() == 0 and Size() == kUwordMax so it includes
> arbitrary PCs.
>
> * In --dwarf-stack-traces mode, most Code objects obtained from stack
> frames are replaced with StubCode::UnknownDartCode().
> This simulates future behavior of ReversePc::Lookup when Code objects
> will be removed.
>
> Issue: https://github.com/dart-lang/sdk/issues/44852
> Change-Id: I7cec7b8b9396c9cfeca3c256a412ba4e82a7e0c4
> TEST=ci
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/182720
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Tess Strickland <sstrickl@google.com>
> Reviewed-by: Martin Kustermann <kustermann@google.com>
Issue: https://github.com/dart-lang/sdk/issues/44852
Change-Id: I6f66171eecf1133363a7ce56193e782e43a20baf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/185488
Reviewed-by: Zach Anderson <zra@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
As part of implementing hot-reload for isolate groups, we have to ensure
reloads on a group are performed using the same state of incremental
compiler.
This means we cannot use a specific isolate's information (such as it's
main port) for reloading purposes. Instead we use the unqiue isolate
group id when communicating with the kernel service.
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=Existing test coverage, future test when hot-reload is fully
implemented with isolate groups.
Change-Id: Ifab39cd2ba689c08507dfab4091cd26951ed54e5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/185320
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The following changes are done in preparation for the removal of Code
objects in AOT with --dwarf-stack-traces:
* Stack trace objects are extended to hold uword PCs (which may not
fit into Smi range).
* Scanning stack frames in GC (StackFrame::VisitObjectPointers)
now avoids using Code objects.
In order to find CompressedStackMaps it now calls
ReversePc::FindCompressedStackMaps.
* Singleton Code object (StubCode::UnknownDartCode()) is prepared as
a replacement for Code objects in stack traces. It has
PayloadStart() == 0 and Size() == kUwordMax so it includes
arbitrary PCs.
* In --dwarf-stack-traces mode, most Code objects obtained from stack
frames are replaced with StubCode::UnknownDartCode().
This simulates future behavior of ReversePc::Lookup when Code objects
will be removed.
Issue: https://github.com/dart-lang/sdk/issues/44852
Change-Id: I7cec7b8b9396c9cfeca3c256a412ba4e82a7e0c4
TEST=ci
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/182720
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This is to ensure that deferred library status is per isolate, rather than per isolate-group.
TEST=deferred_isolate_test.dart
Issue dartbug.com/36097
Change-Id: I1fb8b8f1830aac688a078fe0a82b06c7daa856fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/184040
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Once the vm-service isolate starts up it will iterate over all isolates
to discover already started isolates. It does so via iterating over all
isolate groups and all isolates within them. Doing so requires a read
lock.
Until now it has called into Dart while holding the read lock. This can
lead to a deadlock, since calling Dart code can trigger JIT compilation
which can trigger various things that require the same lock.
Instead, this CL accumulates a list of (isolate-main-port, isolate-name)
pairs while iterating over isolate groups and the isolates therein.
Afterwards it registers those isolates in a bulk - without holding any
locks.
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=Refactoring of existing test suite. Fixes flaky timeouts.
Change-Id: Iea7013ce57cef2500700fb464972104c66357777
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/182321
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This CL changes the semantics of `Pointer<T extends Struct>.ref` to use
the compile-time `T` rather than the runtime `T`.
This enables tree shaking of subtypes of Struct and optimizing `.ref`.
Bug: https://github.com/dart-lang/sdk/issues/38721
TEST=tests/ffi/vmspecific_static_checks_test.dart
TEST=tests/ffi/*struct*test_.dart
Change-Id: Ie19bc3259d1cb721d0ce56d68e82d09dc3a4ad0e
Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try,analyzer-nnbd-linux-release-try,app-kernel-linux-debug-x64-try,dart-sdk-linux-try,front-end-nnbd-linux-release-x64-try,pkg-linux-debug-try,vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/180190
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
This CL changes the semantics of `Pointer<T extends Struct>.ref` to use
the compile-time `T` rather than the runtime `T`.
This enables tree shaking of subtypes of Struct and optimizing `.ref`.
Issue: https://github.com/dart-lang/sdk/issues/38721
TEST=tests/ffi/vmspecific_static_checks_test.dart
TEST=tests/ffi/*struct*test_.dart
Change-Id: I3f5b08c08ec0799ef8aab3c4177e2ac70d25501c
Cq-Include-Trybots: luci.dart.try:vm-precomp-ffi-qemu-linux-release-arm-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try,analyzer-nnbd-linux-release-try,front-end-linux-release-x64-try,front-end-nnbd-linux-release-x64-try,benchmark-linux-try,dart-sdk-linux-try,pkg-linux-release-try,vm-ffi-android-release-arm-try,vm-ffi-android-release-arm64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/177862
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Currently we have things called XPtr which are not what you get from ptr().
Old world:
handle->raw() returns RawObject* (tagged)
raw_obj->ptr() returns RawObject* (untagged)
After 6fe15f6df9:
handle->raw() returns ObjectPtr
obj_ptr->ptr() returns ObjectLayout*
New world:
handle->ptr() returns ObjectPtr
obj_ptr->untag() returns UntaggedObject*
TEST=ci
Change-Id: I6c7f34014cf20737607caaf84979838300d12df2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149367
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
As part of making the compiler and other subsystems independent
of `Isolate` we have to move various state from `Isolate` to
`IsolateGroup`.
The class_table and object_store were already moved to `IsolateGroup`.
This CL only replaces usages of `Isolate::{object_store,class_table}`
with the equivalent in `IsolateGroup`.
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=Pure refactoring - relying on existing test coverage.
Change-Id: I34a0682d715b054d6c5faff077a513980f59a348
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/177126
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
This CL adds support for nested structs in FFI calls, callbacks, and
memory loads and stores through the Struct classes itself.
Nesting empty structs and nesting a structs in themselves (directly or
indirectly) is reported as error.
This feature is almost fully implemented in the CFE transformation.
Because structs depend on the sizes of their nested structs, the structs
now need to be processed in topological order.
Field access to nested structs branches at runtime on making a derived
Pointer if the backing memory of the outer struct was a Pointer or
making a TypedDataView if the backing memory of the outer struct was
a TypedData.
Assigning to a nested struct is a byte for byte copy from the source.
The only changes in the VM are contained in the native calling
convention calculation which now recursively needs to reason about
fundamental types instead of just 1 struct deep.
Because of the amount of corner cases in the calling conventions that
need to be covered, the tests are generated, rather than hand-written.
ABIs tested on CQ: x64 (Linux, MacOS, Windows), ia32 (Linux, Windows),
arm (Android softFP, Linux hardFP), arm64 Android.
ABIs tested locally through Flutter: arm64 iOS.
ABIs not tested: ia32 Android (emulator), x64 iOS (simulator), arm iOS.
TEST=runtime/bin/ffi_test/ffi_test_functions_generated.cc
TEST=runtime/bin/ffi_test/ffi_test_functions.cc
TEST=tests/{ffi,ffi_2}/function_structs_by_value_generated_test.dart
TEST=tests/{ffi,ffi_2}/function_callbacks_structs_by_value_generated_tes
TEST=tests/{ffi,ffi_2}/function_callbacks_structs_by_value_test.dart
TEST=tests/{ffi,ffi_2}/vmspecific_static_checks_test.dart
Closes https://github.com/dart-lang/sdk/issues/37271.
Contains a temporary workaround for
https://github.com/dart-lang/sdk/issues/44454.
Change-Id: I5e5d10e09e5c3fc209f5f7e997efe17bd362214d
Cq-Include-Trybots: luci.dart.try:dart-sdk-linux-try,dart-sdk-mac-try,dart-sdk-win-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-mac-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-nnbd-mac-release-x64-try,vm-kernel-nnbd-win-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-win-release-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,vm-kernel-msan-linux-release-x64-try,vm-kernel-precomp-msan-linux-release-x64-try,vm-kernel-precomp-android-release-arm_x64-try,analyzer-analysis-server-linux-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/169221
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
First, change LocationForPosition and TokenRangeAtLine to return
whether they successfully located the offset/line. Also change the
wrappers in Script (GetTokenLocation and TokenRangeAtLine) to return
whether or not any information was successfully located.
These methods now only change the out parameters in the case that
appropriate information was found, so any unconditional uses of the out
parameters need the out parameters to be appropriately initialized.
We now allow negative lines to be requested in TokenRangeAtLine (with
the failure behavior described above) so the line returned by
LocationAtPosition can be fed into TokenRangeAtLine without intermediate
checking.
The calculation of the token length, which uses the script source and
not the line starts array, has been moved to a new method,
Script::GetTokenLength. The new method returns the length (or a
negative value if the token length could not be determined) instead of
using an out parameter.
TEST=Existing tests on trybots, to ensure tests using current
line/number info aren't affected.
Bug: https://github.com/dart-lang/sdk/issues/44436
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-product-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try
Change-Id: Ibc048a226d11ff9a340a8d249654d07720fdf115
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175365
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
Instead, split each old use into the following cases:
* If the TokenPosition value is expected to be a real token position,
then use TokenPosition::Pos().
* If the TokenPosition is being serialized in some way, then use
TokenPosition::Serialize() and change the place where the
TokenPosition is recreated to use TokenPosition::Deserialize().
* If the value of the TokenPosition is being printed for debugging
purposes, then just use TokenPosition::ToCString() instead.
That is, we try to pin down when token positions are expected to
be real vs. when other types of token positions can be found.
Another source of possible error when using token positions is to
convert between synthetic and real token positions. In the past,
synthetic token positions may have been based off real token positions,
but that is no longer the case. Thus, all methods that allow
that conversion have been removed, and instead there is a new static
method for constructing synthetic tokens from valid nonces.
This CL also makes it so that Pos() and relational operators on token
positions are only defined on real token positions, to avoid any
assumptions about what the value encoded in synthetic positions mean. To
help with cases where non-real token positions may occur, four helper
methods are added:
* TokenPosition::Min(a, b): A static method that returns the smallest
real token position provided. If neither `a` or `b` are real,
returns `a`.
* TokenPosition::Max(a, b): A static method that returns the largest
real token position provided. If neither `a` or `b` are real,
returns `a`.
* TokenPosition::IsWithin(start, end): Determines whether `this` falls
between `start` and `end` (inclusive). If `this` is non-real, then it
must be either `start` or `end` if synthetic, otherwise false.
Otherwise, we mimic the old style of range checking, which means that
non-real starts and ends are treated as less than every real token.
* TokenPosition::CompareForSorting(other): Unlike the relational
operators, provides a comparison between any types of token positions
for purposes such as sorting. Currently only used in the profiler.
It also changes TokenPosition::ToCString() to tag synthetic token
positions, so they can be distinguished from real ones at a glance.
TEST=Existing test suite on trybots, especially the observatory tests
which make heavy use of the debugger and the unit tests for the
profiler/source report modules.
Bug: https://github.com/dart-lang/sdk/issues/44436
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-release-x64-try,vm-kernel-nnbd-linux-release-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-release-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-precomp-linux-product-x64-try
Change-Id: Ic06aa0bc7a1f0fbac7257ed22ca5e7e0ccd7f3f2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/174924
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
This CL adds passing structs by value in FFI trampolines.
Nested structs and inline arrays are future work.
C defines passing empty structs as undefined behavior, so that is not
supported in this CL.
Suggested review order:
1) commit message
2) ffi/marshaller (decisions for what is done in IL and what in MC)
3) frontend/kernel_to_il (IL construction)
4) backend/il (MC generation from IL)
5) rest in VM
Overall architecture is that structs are split up into word-size chunks
in IL when this is possible: 1 definition in IL per chunk, 1 Location in
IL per chunk, and 1 NativeLocation for the backend per chunk.
In some cases it is not possible or less convenient to split into
chunks. In these cases TypedDataBase objects are stored into and loaded
from directly in machine code.
The various cases:
- FFI call arguments which are not passed as pointers: pass individual
chunks to FFI call which already have the right location.
- FFI call arguments which are passed as pointers: Pass in TypedDataBase
to FFI call, allocate space on the stack, and make a copy on the stack
and pass the copies' address to the callee.
- FFI call return value: pass in TypedData to FFI call, and copy result
in machine code.
- FFI callback arguments which are not passed as pointers: IL definition
for each chunk, and populate a new TypedData with those chunks.
- FFI callback arguments which are passed as pointer: IL definition for
the pointer, and copying of contents in IL.
- FFI return value when location is pointer: Copy data to callee result
location in IL.
- FFI return value when location is not a pointer: Copy data in machine
code to the right registers.
Some other notes about the implementation:
- Due to Store/LoadIndexed loading doubles from float arrays, we use
a int32 instead and use the BitCastInstr.
- Linux ia32 uses `ret 4` when returning structs by value. This requires
special casing in the FFI callback trampolines to either use `ret` or
`ret 4` when returning.
- The 1 IL definition, 1 Location, and 1 NativeLocation approach does
not remove the need for special casing PairLocations in the machine
code generation because they are 1 Location belonging to 1 definition.
Because of the amount of corner cases in the calling conventions that
need to be covered, the tests are generated, rather than hand-written.
ABIs tested on CQ: x64 (Linux, MacOS, Windows), ia32 (Linux, Windows),
arm (Android softFP, Linux hardFP), arm64 Android.
ABIs tested locally through Flutter: ia32 Android (emulator), x64 iOS
(simulator), arm64 iOS.
ABIs not tested: arm iOS.
TEST=runtime/bin/ffi_test/ffi_test_functions_generated.cc
TEST=runtime/bin/ffi_test/ffi_test_functions.cc
TEST=tests/{ffi,ffi_2}/function_structs_by_value_generated_test.dart
TEST=tests/{ffi,ffi_2}/function_callbacks_structs_by_value_generated_tes
TEST=tests/{ffi,ffi_2}/function_callbacks_structs_by_value_test.dart
TEST=tests/{ffi,ffi_2}/vmspecific_static_checks_test.dart
Closes https://github.com/dart-lang/sdk/issues/36730.
Change-Id: I474d3a4ee1faadbe767ddadd1b696e24d8dc364c
Cq-Include-Trybots: luci.dart.try:dart-sdk-linux-try,dart-sdk-mac-try,dart-sdk-win-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-mac-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-nnbd-mac-release-x64-try,vm-kernel-nnbd-win-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-win-release-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,vm-kernel-msan-linux-release-x64-try,vm-kernel-precomp-msan-linux-release-x64-try,vm-kernel-precomp-android-release-arm_x64-try,analyzer-analysis-server-linux-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/140290
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This CL enables dart:ffi on Fuchsia.
It has the necessary VM changes for x64 and arm64.
- Allocate VM callback trampoline memory pages with RX.
- Do not use x18 on arm64.
- Save x18 on arm64 in Thread to be able to restore it on exceptions.
We do not have a bot testing this
https://github.com/dart-lang/sdk/issues/38752.
The bot only tests the build, it does not run the emulator.
Manual build and test steps x64 Fuchsia emulator:
```
tools/build.py --os=fuchsia -m debug fuchsia_ffi_test_package
xvfb-run third_party/fuchsia/sdk/linux/bin/femu.sh --image qemu-x64 -N --headless
third_party/fuchsia/sdk/linux/bin/fserve.sh --bucket fuchsia-sdk --image qemu-x64 --device-name step-atom-yard-juicy
third_party/fuchsia/sdk/linux/bin/fpublish.sh out/DebugFuchsiaX64/gen/dart_ffi_test_debug/dart_ffi_test_debug.far
third_party/fuchsia/sdk/linux/bin/fssh.sh --device-name step-atom-yard-juicy run fuchsia-pkg://fuchsia.com/dart_ffi_test_debug#meta/dart.cmx --ignore-unrecognized-flags --packages=/pkg/data/.packages --disable-dart-dev /pkg/data/tests/ffi/all_positive.dart
```
Manual build and test steps arm64 Fuchsia emulator:
```
tools/build.py --no-start-goma --os=fuchsia -m debug -a arm64 fuchsia_ffi_test_package
xvfb-run third_party/fuchsia/sdk/linux/bin/femu.sh --image qemu-arm64 -N --headless --experiment-arm64
third_party/fuchsia/sdk/linux/bin/fserve.sh --bucket fuchsia-sdk --image qemu-arm64 --device-name step-atom-yard-juicy
third_party/fuchsia/sdk/linux/bin/fpublish.sh out/DebugFuchsiaARM64/gen/dart_ffi_test_debug/dart_ffi_test_debug.far
third_party/fuchsia/sdk/linux/bin/fssh.sh --device-name step-atom-yard-juicy run fuchsia-pkg://fuchsia.com/dart_ffi_test_debug#meta/dart.cmx --ignore-unrecognized-flags --packages=/pkg/data/.packages --disable-dart-dev /pkg/data/tests/ffi/all_positive.dart
```
Note that running on the arm64 emulator takes over an hour.
This has been tested on an Estelle device.
TEST=Manual test steps above.
The AOT mode has seen an initial test in g3 by rolling gen_snapshot and
the flutter runner.
Arm64 x18 treatment along the lines of
https://dart-review.googlesource.com/c/sdk/+/119481:
- FfiCallInstr, does not modify x18 at all. We do not have to do
anything special when having a C++ frame on top of a Dart frame.
- NativeEntryInstr and NativeReturnInstr, saves and restores x18 with
PushNativeCalleeSavedRegisters && PopNativeCalleeSavedRegisters. Also
save to the thread field in both places. That way the JumpToFrame stub
can load from the thread field.
Closes: https://github.com/dart-lang/sdk/issues/44434
Change-Id: Ide29369c878fed5909a0d6f7d4c3f7508e179f25
Cq-Include-Trybots: luci.dart.try:vm-fuchsia-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/173273
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This removes our temporary scaffolding support for JIT isolate groups
(which was implemented by creating a new isolate group, loading the
application kernel into it and then merging the heap into the original
isolate group - maintaining a different object store)
It makes all isolates within a group share the same object store, same
libraries and JITed code. It will be conservative to start with, only
allow running unoptimized code, etc.
We will gradually remove the restrictions imposed by this CL:
https://dart-review.googlesource.com/c/sdk/+/173970
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=Tests using --enable-isolate-groups with JIT sharing.
Change-Id: I2bf69a6fe3c905067c4cec2e81613f731c52e5ee
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175302
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Remove the use of Fields, which have since the metadata mechanism was introduced come to require global registration.
Retain some sloppiness matching of declarations by name instead of identity for the sake of hot reload.
TEST=ci
Change-Id: Ifa4cc7724096c606f994b057b7838e124ceb3626
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175141
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
The layout of WeakProperty is defined in C++ instead of Dart. Add missing recognition of its accessors so the compiler can use direct field access instead of always going through natives. Compare LinkedHashMap.
Change the sentinel for WeakProperty lists to object null so that no special constructor is needed.
TEST=ci
Bug: https://github.com/dart-lang/sdk/issues/44333
Change-Id: I8686383053c2c345c972b6615b514a1381e79fda
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175001
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
All existing embedders have been opted into --lazy-async-stacks, the
VM also uses it as it's default in all configurations.
After this CL, any user of --causal-async-stacks will get an error
message when trying to use it.
=> In any such case, please simply remove the flag.
TEST=Exhaustive CQ.
Bug: https://github.com/dart-lang/sdk/issues/37668
Change-Id: Ia440afcf2dba464aa8b8cf381b93bbac8eb9f8dc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/172564
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Clement Skau <cskau@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Since the transition from Source based to Kernel based frontend to the
Dart VM, we have not supported callable redirecting factories. This has
caused all removed code in this CL to be unused and untested!
The support for reifing redirection related information would mainly be
used by embedder / dart:mirrors. Right now its not used by anyone and
the kernel support was never implemented.
I suggest we remove the unused and untested code until a decision has
been made that we actually want to support it - in which case it can be
properly implemented, possibly by re-using some of the deleted code in
this CL.
TEST=CL removes unused code, no test result changes.
Change-Id: I7ed45c85b4efcc1e81ce44cbe08bb555e52101b9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166853
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
The former calls the latter with the same arguments, so we could call
the latter directly, though we prefer to use `_startIsolate()` as the
entrypoint.
This fixes test failures in obfuscated mode, where the runtime cannot
find the `_delayEntrypointInvocation` function.
TEST=Fixes -n dartkp-obfuscate-linux-release-x64 ffi_2/dylib_isolates_test
Change-Id: I062fc16658f90750439f304849daa3911d3a1b89
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171946
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The Isolate.spawn() is part of our dart:isolate library implementation,
it should therefore preferrably live under runtime/lib folder.
This CL does that as well as removing some special message handling
logic by using the new `Dart_RunLoopAsync()` C API, which results in
net removal of code.
As a nice side-effect it also makes isolate spawning slightly faster due
to avoiding extra safepoint/vm<->native transitions.
Issue https://github.com/dart-lang/sdk/issues/44088
TEST=Changes existing implementation, have existing test coverage for it.
Change-Id: I00607a1436946552bbe12e8e23062e8f743d4f11
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171731
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
This is a reland of ade333dd54
Original change's description:
> [wasm] Remove dart:wasm
>
> I'm not entirely sure why I had to modify a random set of fingerprints
> in runtime/vm/compiler/recognized_methods_list.h, but when I did a debug
> build it had some fingerprint errors.
>
> Change-Id: Ib0a0cccb37f2efba509e2b37b6eabe790fa933c4
> TEST= Deleting stuff, so not really necessary
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170620
> Commit-Queue: Liam Appelbe <liama@google.com>
> Reviewed-by: Liam Appelbe <liama@google.com>
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
Change-Id: I0d1388f6312358cc62bb32f5ac7b88a3187223c9
TEST= Deleting stuff, so not really necessary
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171940
Commit-Queue: Ivan Inozemtsev <iinozemtsev@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
This reverts commit ade333dd54.
Reason for revert: This broke both the Flutter HHH and Google3 bots. It looks like some co-ordination is required between the Google3 and Flutter engine build changes for this CL to land.
Original change's description:
> [wasm] Remove dart:wasm
>
> I'm not entirely sure why I had to modify a random set of fingerprints
> in runtime/vm/compiler/recognized_methods_list.h, but when I did a debug
> build it had some fingerprint errors.
>
> Change-Id: Ib0a0cccb37f2efba509e2b37b6eabe790fa933c4
> TEST= Deleting stuff, so not really necessary
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170620
> Commit-Queue: Liam Appelbe <liama@google.com>
> Reviewed-by: Liam Appelbe <liama@google.com>
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
TBR=rmacnak@google.com,liama@google.com
Change-Id: I244d7b3549845dd71719176a62b6ec3a4c4b5f86
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171860
Reviewed-by: Siva Annamalai <asiva@google.com>
I'm not entirely sure why I had to modify a random set of fingerprints
in runtime/vm/compiler/recognized_methods_list.h, but when I did a debug
build it had some fingerprint errors.
Change-Id: Ib0a0cccb37f2efba509e2b37b6eabe790fa933c4
TEST= Deleting stuff, so not really necessary
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170620
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Dart_CreateLightweightIsolate API:
This API can be used by embedders to create a lightweight isolate
(inside an existing isolate group).
It is the analogous to Dart_CreateIsolateGroup - though taking a
parent isolate as parameter (inside whose IG we create a new isolate)
instead of taking kernel/snapshot data.
Right now this API works on AOT and returns an error in JIT, both cases
are covered by the test.
Dart_RunLoopAsync:
The API can be used by embedders to transfer ownership of an isolate to
the VM, which will take care of running the message handling loop and
shuts the isolate down once the last receive port has been closed.
It does allow listening to error/exit events generated by the message
loop implementation as well as allows setting errors-are-fatal,
effectively the same event-loop related parameters from the
`Isolate.spawn()` API, just in Dart.
It's the embedders responsibility to first launch initial dart code
which will take care of responding to events (i.e. directly/indirectly
open a receive port) - otherwise the isolate cannot be talked to and
would immediately shut down.
Since our vm/cc tests do support running in AOT, we use a hybrid
approach to test the functionality: We let a normal Dart test call a
small C wrapper using FFI, to create a lightweight isolate, run it on a
new thread and join that thread.
TEST=vm/dart{,_2}/isolates/dart_api_create_lightweight_isolate_test
Issue https://github.com/dart-lang/sdk/issues/36097
Closes https://github.com/dart-lang/sdk/issues/44088
Change-Id: Id77ba928793fdb517f6cb7e8130df98a0366ddd6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170983
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
When Dart invokes Isolate.spawn the Dart VM calls out to the embedder to
create an isolate (group) or initialize it (in case of lightweight
isolates).
* If the embedder doesn't implement this functionality or fails to
perform the creation/initialization, it will signal this failure
via the return value it gives to the VM in the call back
implementation (i.e. Dart_InitializeParams.create_group and
Dart_InitializeParams.initialize_isolate}).
* If the embeder sucessfully completed it's work, the VM owns the
isolate after the embedder call returns. The VM is then responsible
for running it.
It is an undocumented invariant atm that the embedder is responsible
to make the isolate runnable if it signals the VM that isolate was
sucessfully created/initialized.
=> If it does not do that, we have effectively an isolate leak.
=> Though right now seemingly all of our embedders to that correctly.
To avoid this unintentional isolate leak we make the isolate runnable
ourselves if embedder callback was successful but did not make it
runnable itself.
=> This also avoids unnecessary ceremony code in the embedders that
look like this:
Dart_ExitIsolate();
Dart_IsolateMakeRunnable(child);
Dart_EnterIsolate(child);
The implementation of `Dart_IsolateMakeRunnable()` had an untested and
unused code path that caused running the isolate (on a thread pool).
This is not documented and can lead to bugs because often the
embedders enter the isolate right after making it runnable (e.g. above
code sequence) - which would be racing with the spawned message handler
thread.
=> Furtunately our embedders are well behaved and make the isolate
runnable during the callback.
=> As a safeguard we'll make `Dart_IsolateMakeRunnable()` return an
error if it's called outside the callback (which we can detect by
the presense of a spawn state)
Issue https://github.com/dart-lang/sdk/issues/44088
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=Changes internal details of implementation which is already covered by tests.
Change-Id: Ieef316cc0807d99f2fcb1fbd56bbc9c41e904ba8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170882
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
prevent repeated port allocations
Flutter makes heavy use of short-lived Timers which currently causes
repeated opening/closing of the Timer port. This change allows for
RawReceivePorts to be set active / inactive, allowing for a port to be
kept open but not considered alive.
Flutter benchmark changes:
Before dart-lang/sdk@d5118d5: "notifyListener2_iteration": 545.0
After dart-lang/sdk@d5118d5: "notifyListener2_iteration": 1639.0
With fix: "notifyListener2_iteration": 629.0
Fixes https://github.com/flutter/flutter/issues/69583
TEST=Updated port state tests and verified change against Flutter's microbenchmarks
Change-Id: I50ff1a1b0f1f29bcdac958e41c66429e9586fba2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171628
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Changes Dart_NewWeakPersistentHandle to no longer auto delete the
weak persistent handle.
Changes the signatures of WeakPersistentHandleFinalizers to no longer
have access to the handle.
Flutter PR: https://github.com/flutter/engine/pull/19843
g3 presubmit: cl/318028238
Issue: https://github.com/dart-lang/sdk/issues/42312
TEST=runtime/vm/dart_api_impl_test.cc
Change-Id: I3f77db9954d9486759f903b78c03a494f73c68ba
Cq-Include-Trybots:dart/try:vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,app-kernel-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-android-release-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-msan-linux-release-x64-try,vm-kernel-precomp-msan-linux-release-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-precomp-android-release-arm_x64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,dart-sdk-linux-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try,front-end-linux-release-x64-try,vm-kernel-precomp-win-release-x64-try,vm-kernel-mac-debug-x64-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-nnbd-linux-debug-x64-try,analyzer-nnbd-linux-release-try,front-end-nnbd-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151525
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Clement Skau