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>
Multitests aren't valid Dart files that can be processed by a Dart
implementation so the updater generally does a poor job if it tries to
update one.
It's probably not worth supporting because, in practice, a test should
either be a multitest or a static error test, but not both.
Change the tool to skip over any multitests it encounters. If this
results in it doing nothing at all, it reports that as an error.
Otherwise, it just lists the multitests it didn't process.
Close#37721.
Change-Id: Icfb1ff9fe63f2c249b3ccfba65166b97654a9918
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296760
Commit-Queue: Leaf Petersen <leafp@google.com>
Reviewed-by: Leaf Petersen <leafp@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
This makes the socket benchmark (and real world code also), about 100x faster.
Change-Id: Iaa130b7bf69e4c89d3c3a221680e5b62ffe10583
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296641
Auto-Submit: Jake Macdonald <jakemac@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
In order for an analyzer quick fix to aid the user in correcting
exhaustiveness errors, it needs to know not just the text of the
witness pattern that needs to be inserted, but also which elements are
referenced by the identifiers in that pattern. This will allow it to
add in imports if the user's code doesn't yet import the necessary
library, or insert import prefixes in the case where the user is
already importing the necessary library in a prefixed fashion.
To reduce the effect on the exhaustiveness logic (which was designed
with the intention of building up the witness pattern into a
StringBuffer), we introduce a new type, `DartTemplateBuffer`, which
behaves similarly to StringBuffer but also captures the necessary
semantic information so that the analyzer will be able to fix up
imports in an appropriate way.
This change is mostly isolated to the _fe_analyzer_shared package; it
doesn't add the necessary logic to the analyzer to capture the
elements; it simply plumbs them up to the API boundary between the
packages. Future CLs will add the necessary analyzer logic to make
use of the information.
Change-Id: I7c030fc49a2510acbefd6aeb567e0e7b7102855a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296385
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I3a41bb83767052abda1fdfc77e21bd1a83188482
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296441
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I35d4a7c7ad09cb77ff379af036180794d8b6afbf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296403
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: Idf9012cb8c213b523d1c8bb827e530e0d2cf6609
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296407
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
The latter will factor into a page's alignment offset.
TEST=ci
Change-Id: Iee5117efb3278399e0fd09b21ea92eadc8237296
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/263680
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I4c2840deffe5d790a22facebbcc8a02c1cb98020
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296425
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
State more explicitly that sealed classes are abstract and lead the
user to a better fix with the error message.
Bug: https://github.com/dart-lang/sdk/issues/52073
Change-Id: Id24c6cb187ee5497ca2819f930c48ff5aa8d07fc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296025
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Change-Id: I7192ba0fe31f2059c722559c4acaec4bdf0b01f3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296022
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I3e10209e78e48893d2f2df7f8af7963d319efd9f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296405
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I6218c6ea339b6c19c1495b1db9b2da3fe1654718
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296440
Commit-Queue: Jake Macdonald <jakemac@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
Change-Id: I6564643302f66c5920b38d2269c160099322560b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296421
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
The "@compile-error" comment is an old not-great way of defining static
error tests.
Also fix a test to use a mixin declaration instead of a class for declaring a mixin. This was
broken by class modifiers but the breakage was masked by @compile-error treating a test as
passing if any compile error, even an unrelated one, is reported.
See: https://github.com/dart-lang/sdk/issues/45634
Change-Id: I5d1d387002d886cb87340e2559bf8ed01c3f2d6a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296409
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Nate Bosch <nbosch@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>
TEST=Loaded the response from a getPerfettoVMTimeline request in the
Perfetto trace viewer and made sure everything looked correct.
Change-Id: I8d4bc35fb36601701976c28653db92b2161e4724
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288066
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
TEST=Manually verified that the modified CLI options worked as expected, CI
Change-Id: Ibaa308e5c9c3b96c939e2007ee2eaaca30eae280
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288063
Reviewed-by: Ben Konyi <bkonyi@google.com>
This CL removes the packet_ member in TimelineEventPerfettoFileRecorder
and adds a packet_ member to TimelineEventRecorder. This member will be
used when sending Perfetto traces through the VM Service.
TEST=manually verified that traces recorded with the Perfetto file
recorder still contained correct information, CI
Change-Id: I796369ea0832c265a04e37d91bc09f93967979a8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288061
Reviewed-by: Ben Konyi <bkonyi@google.com>
This will allow us to reuse the shared logic when returning Perfetto
traces through the service. This CL also changes some pointer parameters
to const reference parameters.
TEST=manually verified that traces recorded with the JSON file
recorder still contained correct information, CI
Change-Id: I70ac5183959dd8c6efeb55d4891a8e00d3550ca5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288060
Reviewed-by: Ben Konyi <bkonyi@google.com>
TEST=manually verified that traces recorded with the Perfetto file
recorder still contained correct information, CI
Change-Id: I4bd132dd9e703274ffc262eb7e000e6ee6df4bf8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/287923
Reviewed-by: Ben Konyi <bkonyi@google.com>
TEST=manually verified that traces recorded with the Perfetto file
recorder had correct async track names, CI
Change-Id: Icca1b92d164299140565972b4be8c6c9b7338159
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288021
Reviewed-by: Ben Konyi <bkonyi@google.com>
This supports flow events reported through dart:developer, but does not
support ones reported with Dart_TimelineEvent in dart_tools_api.h. That
will have to be supported in a future CL.
TEST=CI, manually verifying that flow arrows appeared on flow events in
the trace viewer for traces recorded with our Perfetto recorder
Change-Id: Ic75a8ce277b436a609c027c2c0d4e94b51031aa7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280740
Reviewed-by: Stephen Adams <sra@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Leaf Petersen <leafp@google.com>
to handle a method with a diagnostic
Fixes#51198
Change-Id: I6a1921747912318d81121d58c88fccc35d1f809a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/282261
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>
The promote script has been replaced by the promote recipe.
Remove unused code from bot_utils.
Bug: b/277727317
Change-Id: I6a087c11d6e98480d346017c97be82bf23438aed
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296260
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: Jonas Termansen <sortie@google.com>
Change-Id: Ifa93f7329bf988a73e406e7978488a0abbc06ae4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296020
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
Auto-Submit: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Without this, they will run _2 suites by default.
Change-Id: Ica435675c467875a7f3dbab6cf05994835dd0ab5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296583
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
Pulls in a new versions of https://github.com/dart-lang/native with
support for MacOS and enables the tests for these on the MacOS pkg
bots.
Fixes the clang paths in pkg/test_runner/lib/src/options.dart for
mac and win.
Since a relatively recent XCode is available on the bots, use XCode
to build rather than the clang from DEPS.
test/native_toolchain/recognizer_test was refactored to only
run tests for tools installed on the host. So it's unskipped.
Bug: https://github.com/dart-lang/sdk/issues/50565
Change-Id: I3804b01da7a4e74d3e49a1b2ba3c8425132417d7
Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-mac-release-try,pkg-win-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296180
Reviewed-by: Liam Appelbe <liama@google.com>
Auto-Submit: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
Change-Id: Ic8667dd0be2a03c8bb57424904c545b4ec335ccc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296540
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
Commit-Queue: William Hesse <whesse@google.com>
Auto-Submit: Alexander Thomas <athom@google.com>
This bump brings in two false-negative fixes:
* 4c2af04303
* 82e7147cee
These have been tested in google3 and are clean.
Change-Id: I8cba338b91b4b5a383506216b7362352132b48f5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296382
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
In the old scheme, sync* generators would have an entry and a
body. The entry contained typed checks and the body had injected code
to call a 'factory' helper method to construct the _SyncStarIterable.
It was necessary to pass the element type to the body to be passed to
the factory.
int foo(T a, T b) sync* {yield f(a); yield f(b);}
is compiled to something like
foo(a, b) {
T._as(a); T._as(b);
return foo$body(a, b, type$.int);
}
foo$body(a, b, R) {
return _syncStarFactory(function(){BODY}, R);
}
When type checks were disabled (`-O3`), it was often possible to
generate a single function by merging these as there were no checks.
The new scheme keeps the entry and body separate, and constructs the
_SyncStarIterable in the entry:
foo(a, b) {
T._as(a); T._as(b);
return _syncStarFactory(foo$body(a, b), type$.int);
}
foo$body(a, b) {
return function(){BODY};
}
This keeps the typed Dart 'level' distinct from the untyped JavaScript
level.
The new scheme is a bit more verbose but has the advantage that the
call to `_syncStarFactory` can be inlined and optimized.
Change-Id: I13802d9c9eefd9323841670d059b75a81569d6cb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296140
Reviewed-by: Mayank Patke <fishythefish@google.com>
Commit-Queue: Stephen Adams <sra@google.com>
