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>
Decorating a field with 'vm:shared' pragma makes values in this field accessible to all isolates in an isolate group.
Introduce `channel` to the `Version` class so that the pragma can only be enabled on main and dev channels.
TEST=shared_test, shared_fail_without_flag_test
BUG=https://github.com/dart-lang/sdk/issues/55991
Change-Id: I843c9f0d2ffc9f2ced7ddc4006bb6f9ca4e2ddf4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370064
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
This reverts commit bc0f02e4c8.
Reason for revert: https://github.com/dart-lang/sdk/issues/55754
Original change's description:
> [vm, gc] Incremental compaction.
>
> At the beginning of a major GC cycle, select some mostly-empty pages to be evacuated. Mark the pages and the objects on these pages. Apply a write barrier for stores creating old -> evacuation candidate pointers, and discover any such pointers that already exist during marking.
>
> At the end of a major GC cycle, evacuate objects from these pages. Forward pointers of objects in the remembered set and new-space. Free the evacuated pages.
>
> This compaction is incremental in the sense that creating the remembered set is interleaved with mutator execution. The evacuation step, however, is stop-the-world.
>
> Write-barrier elimination for x.slot = x is removed. Write-barrier elimination for x.slot = constant is removed in the JIT, kept for AOT but snapshot pages are marked as never-evacuate.
>
> TEST=ci
> Bug: https://github.com/dart-lang/sdk/issues/52513
> Change-Id: Icbc29ef7cb662ef8759b8c1d7a63b7af60766281
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357760
> Reviewed-by: Alexander Aprelev <aam@google.com>
> Commit-Queue: Ryan Macnak <rmacnak@google.com>
Bug: https://github.com/dart-lang/sdk/issues/52513
Change-Id: I565ad6c0fca283d33f605c10f181bc0a59e7d2b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366965
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
Auto-Submit: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
At the beginning of a major GC cycle, select some mostly-empty pages to be evacuated. Mark the pages and the objects on these pages. Apply a write barrier for stores creating old -> evacuation candidate pointers, and discover any such pointers that already exist during marking.
At the end of a major GC cycle, evacuate objects from these pages. Forward pointers of objects in the remembered set and new-space. Free the evacuated pages.
This compaction is incremental in the sense that creating the remembered set is interleaved with mutator execution. The evacuation step, however, is stop-the-world.
Write-barrier elimination for x.slot = x is removed. Write-barrier elimination for x.slot = constant is removed in the JIT, kept for AOT but snapshot pages are marked as never-evacuate.
TEST=ci
Bug: https://github.com/dart-lang/sdk/issues/52513
Change-Id: Icbc29ef7cb662ef8759b8c1d7a63b7af60766281
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357760
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Now Slots have an extra flag that describes whether or not the
instance object for that Slot is tagged (Dart objects) or untagged
(non-Dart objects like Thread).
Add new Slot categories for Slots with untagged instances, based on
the return value of the Slot:
* a tagged value (e.g., the finalizers_ field of Isolate),
* an untagged value (e.g., the isolate_ field of Thread), or
* an unboxed value (e.g., the enabled_ field of StreamInfo).
The primary difference in the specification of slots for untagged
instances vs tagged instances is that the slots for untagged instances
do not include an underlying type (as only Dart objects have an
Untagged<X> for each <X> class that specifies the actual object layout).
Also do a little more refactoring of Slot categories to simplify
some switch statements that now only need to be updated if the
categories the switch statement deals with change. For example,
methods for range analysis are only interested in Slots that are
guaranteed to contain integers, so now those methods are written in
a way that they only need to be updated if a new tagged integer
Slot declaration is added.
Adjust LoadFromSlot, StoreToSlot, and StoreToSlotNoBarrier to
appropriately handle Slots on untagged instances, and change
LoadField and StoreField's RequiredInputRepresentation methods
to appropriately returned kTagged or kUntagged based on the slot.
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-release-ia32-try
Change-Id: Ic27de4f092c9e087486066459f68a82b4e1685b0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357341
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
- Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
- A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
- Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
- Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
- reachabilityBarrier is now just a count of major GCs.
TEST=ci
Change-Id: I1e653c9b5d3e02e45b280302c832157a75788db6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345350
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
This reverts commit a3b7c323b0.
Reason for revert: We are seeing crashes in some internal testing
si_signo=Segmentation fault(11), si_code=SEGV_ACCERR(2), si_addr=0x7f1cadb85bc6
version=3.3.0-edge+google3-v2 (google3) (google3) on "linux_x64"
pid=8967, thread=8969, isolate_group=main(0x5583571aaaf0), isolate=(nil)((nil))
os=linux, arch=x64, comp=no, sim=no
isolate_instructions=7f1cad8b7e40, vm_instructions=7f1cad8b1000
fp=7f1cacc7ebc0, sp=7f1cacc7eb90, pc=5583568155bc
pc 0x00005583568155bc fp 0x00007f1cacc7ebc0 dart::MarkingVisitorBase<true>::VisitPointers(dart::ObjectPtr*, dart::ObjectPtr*)+0x15c
pc 0x0000558356975a1d fp 0x00007f1cacc7ec60 dart::StackFrame::VisitObjectPointers(dart::ObjectPointerVisitor*)+0x1bd
pc 0x0000558356935c29 fp 0x00007f1cacc7ecc0 dart::UntaggedSuspendState::VisitSuspendStatePointers(dart::SuspendStatePtr, dart::ObjectPointerVisitor*)+0x89
pc 0x0000558356815948 fp 0x00007f1cacc7ed10 dart::MarkingVisitorBase<true>::DrainMarkingStackWithPauseChecks()+0x128
pc 0x0000558356815749 fp 0x00007f1cacc7ed80 dart::ConcurrentMarkTask::Run()+0x69
Original change's description:
> [vm, gc] Mark through new-space.
>
> - Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
> - A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
> - Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
> - Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
> - reachabilityBarrier is now just a count of major GCs.
>
> TEST=ci
> Change-Id: Ic7754e8d972763654eae2b7faa8670735d9cda3f
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/340644
> Reviewed-by: Siva Annamalai <asiva@google.com>
> Commit-Queue: Ryan Macnak <rmacnak@google.com>
Change-Id: Ice07b4eb5bef3b41c9618ef0ca7759de006ffe00
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/343060
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Siva Annamalai <asiva@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
STW marking is no longer O(new-space), so there's no longer a reason to delay finalizing marking hoping for a scavenge to make new-space mostly empty.
TEST=ci
Change-Id: Ie782e88852714d30e0c75aa9aecac62e56c434ce
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319880
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
- Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
- A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
- Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
- Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
- reachabilityBarrier is now just a count of major GCs.
TEST=ci
Change-Id: Ic7754e8d972763654eae2b7faa8670735d9cda3f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/340644
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This reverts commit 5daaa7d9eb.
Reason for revert: internal crashes
Original change's description:
> [vm, gc] Mark through new-space.
>
> - Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
> - A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
> - Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
> - Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
> - reachabilityBarrier is now just a count of major GCs.
>
> TEST=ci
> Change-Id: I4a6a23273d8ecb78c640f054731d4ceb737bfc4d
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/325840
> Reviewed-by: Siva Annamalai <asiva@google.com>
> Commit-Queue: Ryan Macnak <rmacnak@google.com>
Change-Id: I8a50074db343c63c14f0487ae8b4f5fee2c4ae76
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330720
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
- Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
- A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
- Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
- Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
- reachabilityBarrier is now just a count of major GCs.
TEST=ci
Change-Id: I4a6a23273d8ecb78c640f054731d4ceb737bfc4d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/325840
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This is follow up for the previous change somewhat incorrectly used
FLAG_profiler in ~OSThread to determine whether we need to delete
interrupter related state. FLAG_profiler is mutable in non-PRODUCT
builds so this could create memory leak.
TEST=testing PRODUCT and non-PRODUCT builds manually
Change-Id: Icf5ca6b83daab91daa125755261700ba2dbb9533
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/328422
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This reverts commit 3fb88e4c66.
Reason for revert: b/297175670
Original change's description:
> [vm, gc] Mark through new-space.
>
> - Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
> - A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
> - Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
> - Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
> - reachabilityBarrier is now just a count of major GCs.
>
> TEST=ci
> Change-Id: I6362802cd93ba5ba9c39f116ddff82e4feb4c312
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/321304
> Commit-Queue: Ryan Macnak <rmacnak@google.com>
> Reviewed-by: Siva Annamalai <asiva@google.com>
Change-Id: I33075156160dc35861355d738a5776b74dce88b9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/322344
Reviewed-by: Martin Kustermann <kustermann@google.com>
Auto-Submit: Ivan Inozemtsev <iinozemtsev@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
- Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
- A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
- Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
- Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
- reachabilityBarrier is now just a count of major GCs.
TEST=ci
Change-Id: I6362802cd93ba5ba9c39f116ddff82e4feb4c312
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/321304
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
This reverts commit e95e7b8e96.
Reason for revert: suspected cause to non-deterministic AOT binaries crashes, b/296014654
Original change's description:
> [vm, gc] Mark through new-space.
>
> - Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
> - A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
> - Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
> - Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
> - reachabilityBarrier is now just a count of major GCs.
>
> TEST=ci
> Change-Id: I8c2c64b120766571b62d3bd8dab37ae81c2dca98
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319583
> Commit-Queue: Ryan Macnak <rmacnak@google.com>
> Reviewed-by: Siva Annamalai <asiva@google.com>
Change-Id: Idda542c7c657d4f14c836423b173c9b067132212
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/320820
Reviewed-by: Martin Kustermann <kustermann@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Ilya Yanok <yanok@google.com>
- Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
- A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
- Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
- Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
- reachabilityBarrier is now just a count of major GCs.
TEST=ci
Change-Id: I8c2c64b120766571b62d3bd8dab37ae81c2dca98
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319583
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
This reverts commit 6194209b28.
Reason for revert: issues on arm32
Original change's description:
> [vm, gc] Mark through new-space.
>
> - Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
> - A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
> - Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
> - Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
> - reachabilityBarrier is now just a count of major GCs.
>
> TEST=ci
> Change-Id: I3668a2e56821f9eadf96e38c228dab27be656016
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309826
> Reviewed-by: Siva Annamalai <asiva@google.com>
> Commit-Queue: Ryan Macnak <rmacnak@google.com>
Change-Id: I434eb595c9e7858efc8c9b07cbca954e5649f506
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319321
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
- Initial and final marking no longer visit all of new-space, reducing the STW pause for major GC.
- A scavenge during concurrent marking must forward / filter objects in the marking worklist that are moved / collected, increasing the STW pause for minor GC.
- Unreachable intergenerational cycles and weak references are collected in the next mark-sweep instead of first requiring enough scavenges to promote the whole cycle or weak target into old-space.
- Artificial minor GCs are no longer needed to avoid memory leaks from back-to-back major GCs.
- reachabilityBarrier is now just a count of major GCs.
TEST=ci
Change-Id: I3668a2e56821f9eadf96e38c228dab27be656016
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309826
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This is a revert of b6bbfff8c0c6c44cdab0d7d78 which temporarily disabled
this.
Turns out that a non-TSAN gen_snapshot would - when emitting code via
`Assembler::TsanLoadAcquire` - use an incorrect `Thread`-offset:
It calculated the offset via `Thread::OffsetFromThread`. That function
took a `dart::RuntimeEntry*` and tried to find its offset.
We happen to have the following leaf runtime entries:
#define LEAF_RUNTIME_ENTRY_LIST(V)
...
V(void, MsanUnpoison, void*, size_t)
V(void, MsanUnpoisonParam, size_t)
V(void, TsanLoadAcquire, void*)
V(void, TsanStoreRelease, void*)
...
It loops over all runtime entries and finds the first one that has the
identical `dart::RuntimeEntry::function_` pointer.
Though all 4 of them are `nullptr` at `gen_snapshot` time, so when
searching for offset for
`Thread::OffsetFromThread(kTsanLoadAcquireRuntimeEntry)`
it looked for the first runtime entry with `nullptr` function pointer -
which turned out to be `MsanUnpoison` (instead of `TsanLoadAcquire`).
=> The obvious fix is to use the `dart::RuntimeEntry*` pointer for
comparison instead of it's `function_` member.
TEST=ci
Issue b/287638965
Change-Id: I85c06674927978ef8561e9e7bdfab4823c0a8e1c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312902
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
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>
This splits `Dart::InitializeIsolate()` into
* `Dart::InitializeIsolateGroup()` that sets up an `IsolateGroup`
=> This is only done at isolate group creation time when the very
first isolate is created.
* `Dart::InitializeIsolate()` that sets up an `Isolate`.
=> This is done for every isolate.
This is purely refactoring / code cleanup.
TEST=ci
Change-Id: Ica906444f79fe49849b9e11e96f7c89184cb9d09
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311603
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This is the same metadata system that will support async callbacks. The
main thing this does is take the trampolines that used to be JIT only,
and use them in AOT too. This is partly a safety thing (there's some
extra checks that used to be skipped on AOT), but mostly just so that
the metadata system is unified between the sync and async callbacks.
More details about the design:
https://docs.google.com/document/d/1QDjyY_6wOTOgURwpeYMKU9qEz0gKxx2MUrdruC6Kp6c/edit?usp=sharing
I split this off the async CL. Some of the comments refer to async
stuff that doesn't exist yet, but it's coming immediately after this so
I didn't update them.
Change-Id: Icd5e86934ee9ae34c2c0e2ed2bbd1b928a7184ac
TEST=ffi_callback_metadata_test.cc and CI
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302903
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Threads that own safepoint operations (e.g. a thread owning
[ReloadSafepointOperation]) are free to exit & re-enter. Owning a
safepoint operation only means that other threads are at well defined
places.
We limit the number of active mutators that can be running at the same
time - mainly due to the way our GC works today. This is maintained by
threads blocking on entering when there's too many mutators already.
If a thread owns a safepoint operation and exits, it should not give up
it's mutator slot to ensure it will be able to re-enter.
A concrete case when this can happen: We have many isolates and we run
in `--hot-reload-test-mode`. One isolate will own a reload safepoint
operation & perform reload. As part of reload it will exit the isolate
and send a request to the `kernel-service` and wait for it's reply. Once
it gets a replay it will re-enter the isolate. Now this re-entering
could be blocking if another thread took the mutator slot (which can
happen, as we get mutator slot before we try to check-in to safepoint
when entering).
In the future we may unify the safepoint mechanism with the mutator
count mechanism.
Closes https://github.com/dart-lang/sdk/issues/52441
TEST=Fixes flaky timeouts of ffi/invoke_callback_after_suspension_test
Change-Id: Icc5dbf59b4270653c9e6e316531f5b3e086db2fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304682
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The ffi-callback related information on the [Thread] object is metadata
corresponding to ffi-callback-trampoline [Function] objects. There is
nothing thread or isolate specific about it.
Moving it away from [Thread] is needed because an [Isolate] can
have different [Thread] objects across its lifetime (see [0]): When
the stack of an isolate is empty, we reserve now the right to
re-cycle the [Thread]. If the isolate later runs again, it may
get a new [Thread] object.
This CL moves this information from [Thread] to the [ObjectStore]. In
addition we make the compiler be responsible for populating this
metadata - instead of doing this per-call site of
`Pointer.fromFunction()`. It will be preserved across snapshot writing
& snapshot reading (for AppJIT as well as AppAOT).
Similarly the JIT trampolines that are on Isolate aren't isolate
specific and can go to [IsolateGroup]. This simplifies doing the above
as the compiler can allocate those as well.
The effect is that [Thread] object gets smaller, GC doesn't have to
visit the 2 slots per-thread. It comes at expense of 2 more loads
when invoking the callback.
[0] https://dart-review.googlesource.com/c/sdk/+/297920
TEST=Regression test is vm/ffi{,_2}/invoke_callback_after_suspension_test
Change-Id: Ifde46a9f6e79819b5c0e359c3d3998d1d93b9b1e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303700
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: 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 reload safepoint operation mechanism will send out-of-band message
to all isolates that are not already at-reload-safepoint.
It identified all such threads by iterating the active threads list,
filtering those that are not at-reload-safepoint and those with an
active isolate (i.e. `thread->isolate() != nullptr`).
This `thread->isolate() != nullptr` condition isn't quite
correct. A mutator may temporarily run under a [NoActiveIsolateScope] to
make the `Isolate::Current()` unavailable to code that shouldn't depend
on isolates (e.g. GC and Compiler do that) or may even have the
incorrect isolate on it (e.g. Debugger Service notifications).
So if one thread triggers a [ReloadSafepointOperation] and another
thread is under a [NoActiveIsolateScope] it will not get the OOB message
and therefore not get interrupted to check-in.
This has caused flaky timeouts of `vm/dart/isolates/reload_active_stack_test`
- as this test runs isolates with active stack without going back
to event loop (which would check into reload operations).
Closes https://github.com/dart-lang/sdk/issues/52135
TEST=Updated vm/cc/Reload_NotAtSafepoint for regression test
TEST=Fixes flaky timeouts of vm/dart/isolates/reload_active_stack_test
Change-Id: Ib407c42aa97798ac994aff3bce263da79b83666a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302320
Reviewed-by: Slava Egorov <vegorov@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>
The sweeper threads are bypassing safepoints and have no need for normal
mutator state (e.g. storebuffer, marking stacks, .,..) on the [Thread]
object.
This fixes a TSAN report where sweeper would clear reusable handles
(which it didn't actually modify) and scavenger is reading those
handles (which arguably it doesn't have to either).
Issue https://github.com/dart-lang/sdk/issues/52125
TEST=ci
Change-Id: I03a36e8518b6c00eb7f3b57f65fd469dddba23e0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296860
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>
Some cleanups factored out of a larger CL (which refactors enter/exit of threads):
* remove unused `#include "vm/thread_registry.h"`
* remove unused/unnecessary fields from [Thread] object
* rename IsMutator() -> IsDartMutator()
* make tests using setjmp() drain the sticky error
=> to ensure there's no sticky error on isolate shutdown
TEST=ci
Change-Id: I53935e8bd0628ab3768627d6d5e01c3f0e3a57ad
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296582
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
For every isolate there should be only one mutator with
a unique [Thread] object.
We change existing tests that use this functionality to instead use
`Thread::{Enter,Exit}IsolateGroupAsHelper`. It also results in a net
removal of code.
TEST=ci
Change-Id: Ic326e868a98ddedbab5b8c429252d38ea71bbf04
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/295940
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Alexander Markov