This reland commit 5a32d8bc7c with a fix
for thread leak (Issue #56717): when `ThreadPool` is shutting down
asynchronously the last worker should detach itself to prevent
leaking associated low-level data structures, because no thread will
join it.
A hang in service isolate shutdown (caused by an existing bug) was fixed by commit 157a0dc7f9.
This CL turns native ports into a thin abstraction over underlying
thread pool instead of building them as full fledged MessageHandler.
This allows to easily implement a variation of native ports which can
handle messages concurrently with the given degree of concurrency.
This type of port can be used to greatly simplify implementation of
IOService - which previously had to do its own concurrency management
on top of "single threaded" native ports. This capability is exposed
as `Dart_NewConcurrentNativePort` API.
The new implementation is in general much cleaner then the old one
with one exception: `Dart_CloseNativePort` API has unfortunate design
where underlying message handler is destroyed asynchronously and
`Dart_CloseNativePort` returns immediately without waiting for pending
tasks to complete. Implementing this on top of `ThreadPool` requires
some changes to thread pool implementation.
Issue https://github.com/dart-lang/sdk/issues/55844
Closes https://github.com/dart-lang/sdk/issues/56717
TEST=ci
Change-Id: Ic68bfb60757685afd75c80a70cdec66cc13c149b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/385000
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This reverts commit 5a32d8bc7c.
Reason for revert: DartIsolateTest.CanCreateServiceIsolate flutter engine unit test hangs on Windows.
Original change's description:
> [vm] Simplify implementation of native ports
>
> This CL turns native ports into a thin abstraction over underlying
> thread pool instead of building them as full fledged MessageHandler.
>
> This allows to easily implement a variation of native ports which can
> handle messages concurrently with the given degree of concurrency.
> This type of port can be used to greatly simplify implementation of
> IOService - which previously had to do its own concurrency management
> on top of "single threaded" native ports. This capability is exposed
> as `Dart_NewConcurrentNativePort` API.
>
> The new implementation is in general much cleaner then the old one
> with one exception: `Dart_CloseNativePort` API has unfortunate design
> where underlying message handler is destroyed asynchronously and
> `Dart_CloseNativePort` returns immediately without waiting for pending
> tasks to complete. Implementing this on top of `ThreadPool` requires
> some changes to thread pool implementation.
>
> Issue https://github.com/dart-lang/sdk/issues/55844
>
> TEST=ci
>
> Change-Id: I062040ff233e93962ae93684e9b044d8facdaffc
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/382163
> Commit-Queue: Slava Egorov <vegorov@google.com>
> Reviewed-by: Martin Kustermann <kustermann@google.com>
Change-Id: I7de27793a54072e974bf1a9f17a07c12159a202d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384481
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This CL turns native ports into a thin abstraction over underlying
thread pool instead of building them as full fledged MessageHandler.
This allows to easily implement a variation of native ports which can
handle messages concurrently with the given degree of concurrency.
This type of port can be used to greatly simplify implementation of
IOService - which previously had to do its own concurrency management
on top of "single threaded" native ports. This capability is exposed
as `Dart_NewConcurrentNativePort` API.
The new implementation is in general much cleaner then the old one
with one exception: `Dart_CloseNativePort` API has unfortunate design
where underlying message handler is destroyed asynchronously and
`Dart_CloseNativePort` returns immediately without waiting for pending
tasks to complete. Implementing this on top of `ThreadPool` requires
some changes to thread pool implementation.
Issue https://github.com/dart-lang/sdk/issues/55844
TEST=ci
Change-Id: I062040ff233e93962ae93684e9b044d8facdaffc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/382163
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
When uploading CLs, the presubmit checks verify that the lines in the
diff are formatted correctly according to `git cl format runtime`.
However, when `buildtools/<os>-<arch>/clang/bin/clang-format` is
updated, it does not force reformatting of files that would be
reformatted.
This leads to two issues:
* Inconsistent style within the code base and within a single file.
* Spurious reformatting in CLs when (1) clang-format is used on the
whole file, or (2) the diff lines overlap.
`clang-format` doesn't change that frequently, so in general this is
not a large issue, but I've seen a bit too many "spurious formatting,
please revert" comments on CLs recently.
This CL formats the runtime to be in line with the current pinned
`clang-format`:
```
$ find runtime/ -iname *.h -o -iname *.cc | xargs buildtools/mac-arm64/clang/bin/clang-format -i
```
`git cl format` (which only formats changed lines, and does so with
`clang-format`) seems to not agree with itself, or clang-format, or
cpplint in a handful of places. This CL adds `// clang-format off`
for these. (See previous patchsets for the specific instances.)
TEST=A variety of bots including GCC, MacOS and Windows.
Change-Id: I470892e898971899fda14bb3b8f2c8efefd67686
Cq-Include-Trybots: luci.dart.try:vm-gcc-linux-try,vm-ffi-qemu-linux-release-riscv64-try,vm-ffi-qemu-linux-release-arm-try,vm-aot-win-debug-x64-try,vm-win-debug-x64c-try,vm-mac-debug-x64-try,vm-mac-debug-arm64-try,vm-aot-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/362780
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Patchset 1 is a reland of commit
f52bdb96aa, and Patchset 2 adds a
corrected reland of commit 0d0e301979.
The leak in g3 was caused by moving the `state_ = 0;` line before the
`owns_label()` check in `TimelineEvent::Reset`.
TEST=Ran the g3 tests that were failing on a cbuild copy with these
changes (sponge2/1134a5bb-5168-4e4c-bb03-72a7d7ad6ab4).
Original change's description:
> [VM/Timeline] Add Dart_RecordTimelineEvent Embedder API to support reporting flow events that can be serialized in Perfetto's format
>
> Added a deprecation notice to Dart_TimelineEvent as well.
>
> TEST=Used the embedder API to report an events of each of the following types:
> Dart_Timeline_Event_Begin, Dart_Timeline_Event_Duration,
> Dart_Timeline_Event_Instant, Dart_Timeline_Event_Async_Begin, and
> Dart_Timeline_Event_Async_Instant, each associated with flow IDs. Then,
> retrieved the trace using GetPerfettoVMTimeline and checked that flow arrows
> were displayed correctly in the Perfetto trace viewer.
>
> Change-Id: I82e49554b5970767e306dc4149a782e99a5c6512
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302963
> Commit-Queue: Derek Xu <derekx@google.com>
> Reviewed-by: Ben Konyi <bkonyi@google.com>
Change-Id: I08783d6b5575fb3fe43da3203e58902bf9b7d038
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/308721
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
This reverts commit f52bdb96aa.
Reason for revert: https://dart-review.googlesource.com/c/sdk/+/308360
was reverted due to g3 problems, and the code in this CL will leak
without the code in that CL.
Original change's description:
> [VM/Timeline] Add Dart_RecordTimelineEvent Embedder API to support reporting flow events that can be serialized in Perfetto's format
>
> Added a deprecation notice to Dart_TimelineEvent as well.
>
> TEST=Used the embedder API to report an events of each of the following types:
> Dart_Timeline_Event_Begin, Dart_Timeline_Event_Duration,
> Dart_Timeline_Event_Instant, Dart_Timeline_Event_Async_Begin, and
> Dart_Timeline_Event_Async_Instant, each associated with flow IDs. Then,
> retrieved the trace using GetPerfettoVMTimeline and checked that flow arrows
> were displayed correctly in the Perfetto trace viewer.
>
> Change-Id: I82e49554b5970767e306dc4149a782e99a5c6512
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302963
> Commit-Queue: Derek Xu <derekx@google.com>
> Reviewed-by: Ben Konyi <bkonyi@google.com>
Change-Id: I98a259c7de8fa4b7dbcbe42f7332ab20b553e9cc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/308680
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Derek Xu <derekx@google.com>
Added a deprecation notice to Dart_TimelineEvent as well.
TEST=Used the embedder API to report an events of each of the following types:
Dart_Timeline_Event_Begin, Dart_Timeline_Event_Duration,
Dart_Timeline_Event_Instant, Dart_Timeline_Event_Async_Begin, and
Dart_Timeline_Event_Async_Instant, each associated with flow IDs. Then,
retrieved the trace using GetPerfettoVMTimeline and checked that flow arrows
were displayed correctly in the Perfetto trace viewer.
Change-Id: I82e49554b5970767e306dc4149a782e99a5c6512
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302963
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This is a reland of commit 46e70a7c07
It turns out that flow events do need to be associated with IDs in
Chrome's trace format, so the diff between patchset 1 and 6 contains the
changes required to document that.
TEST=Checked that duration events still looked correct in traces written
by the Perfetto file recorder. Checked that events still looked correct
in traces recorded to the MacOS recorder. Checked that flow events still
looked correct in Observatory. Ran the
vm/cc/DartAPI_TimelineEvents_Serialization test in debug mode, and with
ASAN and TSAN.
CoreLibraryReviewExempt: Does not change any APIs, just stops passing a
value that will never be read. Specifically, we do not need to pass a
flow ID on the end side of track events.
Change-Id: I6b8bb3236339dbe54378b2e9dea06faf22b964a9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306664
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
This reverts commit 46e70a7c07.
Reason for revert: New asserts failing in Flutter Engine tests
Original change's description:
> Reland "[VM/Timeline] Make timestamp1_or_id variables more understandable"
>
> Relanding without changes because the cbuild failure was unrelated.
>
> TEST=Checked that duration events still looked correct in traces written
> by the Perfetto file recorder. Checked that events still looked correct
> in traces recorded to the MacOS recorder.
>
> This is a reland of commit 354e1d22cb
>
> Original change's description:
> > [VM/Timeline] Make timestamp1_or_id variables more understandable
> >
> > This CL improves documentation, and renames fields/methods to make it
> > more understandable that timestamp1 and id are stored in the same field
> > of dart::TimelineEvent (because the event types that need to store
> > timestamp1 are disjoint from the ones that need to store id).
> >
> > TEST=Checked that duration events still looked correct in traces written
> > by the Perfetto file recorder. Checked that events still looked correct
> > in traces recorded to the MacOS recorder.
> >
> > Change-Id: I25ff1f4c6cc432f035ac2be99fa7f162290ea77f
> > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305880
> > Reviewed-by: Ben Konyi <bkonyi@google.com>
> > Commit-Queue: Derek Xu <derekx@google.com>
>
> Change-Id: I28c0c3af9869900cc8bb2b3b416904a231d143e0
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306301
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Commit-Queue: Derek Xu <derekx@google.com>
Change-Id: Ia1fe524268c06fd68c689fe048d8cc38a6ed8a23
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306661
Reviewed-by: Ben Konyi <bkonyi@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Derek Xu <derekx@google.com>
Relanding without changes because the cbuild failure was unrelated.
TEST=Checked that duration events still looked correct in traces written
by the Perfetto file recorder. Checked that events still looked correct
in traces recorded to the MacOS recorder.
This is a reland of commit 354e1d22cb
Original change's description:
> [VM/Timeline] Make timestamp1_or_id variables more understandable
>
> This CL improves documentation, and renames fields/methods to make it
> more understandable that timestamp1 and id are stored in the same field
> of dart::TimelineEvent (because the event types that need to store
> timestamp1 are disjoint from the ones that need to store id).
>
> TEST=Checked that duration events still looked correct in traces written
> by the Perfetto file recorder. Checked that events still looked correct
> in traces recorded to the MacOS recorder.
>
> Change-Id: I25ff1f4c6cc432f035ac2be99fa7f162290ea77f
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305880
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Commit-Queue: Derek Xu <derekx@google.com>
Change-Id: I28c0c3af9869900cc8bb2b3b416904a231d143e0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306301
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
This reverts commit 354e1d22cb.
Reason for revert: broke cbuild
Original change's description:
> [VM/Timeline] Make timestamp1_or_id variables more understandable
>
> This CL improves documentation, and renames fields/methods to make it
> more understandable that timestamp1 and id are stored in the same field
> of dart::TimelineEvent (because the event types that need to store
> timestamp1 are disjoint from the ones that need to store id).
>
> TEST=Checked that duration events still looked correct in traces written
> by the Perfetto file recorder. Checked that events still looked correct
> in traces recorded to the MacOS recorder.
>
> Change-Id: I25ff1f4c6cc432f035ac2be99fa7f162290ea77f
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305880
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Commit-Queue: Derek Xu <derekx@google.com>
Change-Id: I37c74620105299c0b5e7725fd994609af177a9f8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306280
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Derek Xu <derekx@google.com>
This CL improves documentation, and renames fields/methods to make it
more understandable that timestamp1 and id are stored in the same field
of dart::TimelineEvent (because the event types that need to store
timestamp1 are disjoint from the ones that need to store id).
TEST=Checked that duration events still looked correct in traces written
by the Perfetto file recorder. Checked that events still looked correct
in traces recorded to the MacOS recorder.
Change-Id: I25ff1f4c6cc432f035ac2be99fa7f162290ea77f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305880
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
This simplifies embedders which otherwise need to maintain
special mappings to find their isolate specific data structures.
TEST=vm/cc/DartAPI_SetTimelineRecorderCallback
Change-Id: If819437cad2e1bf3fe5ba50fe67d01e8bd992064
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304962
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
The only user of this API is no longer using it (see cl/519087228).
It's possible we'd re-introduce such an API in the future,
in which case we may want to:
- make it more clear what the GC times mean (are concurrent threads for
mark/sweep or parallel threads for paralell marking included,
is time in wallclock or cpu time, ...)
- include information more useful for users, such as maximum
synchronous pause times
TEST=ci
Change-Id: Idec6f1fdc406956889dd858f280dfe84bed0d10f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291260
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Allows embedders to programmatically request heap snapshot from a
running isolate group instead of relying on a vm-service or hidden
internal Dart APIs
Additionally we allow to include snapshot writing functionality into
PRODUCT builds using DART_ENABLE_HEAP_SNAPSHOT_WRITER define.
TEST=vm/cc/DartAPI_WriteHeapSnapshot
Bug: b/259115846
Change-Id: Ic3ef76e5fb9adcf8f23a1959f5238742b64ecde2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/273181
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
Adds the helper functions HasIsolateId(), HasIsolateGroupId(),
GetFormattedIsolateId(), and GetFormattedIsolateGroupId() to
timeline.h
The Perfetto recorder that will be added in a future CL will make use of
these.
TEST=ci
Change-Id: I50e234d9c2cc551ae9fbad7e92fc6e019213e264
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/259220
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
not used anywhere
Enabling of the timeline streams is done using command line flags
during initialization and no embedder uses this API function
(It was added when Mojo was supported).
TEST=ci
Change-Id: Ide9205b9ecd703e2ccdab7a85e8a79c48d98f180
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209850
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Siva Annamalai <asiva@google.com>
events on Developer stream.
Also removes intrinsic implementations for UserTag_makeCurrent to allow
for service events to be sent on UserTag change.
TEST=DartAPI_UserTags,pkg/vm_service/test/user_tag_changed_test.dart
Change-Id: I5dc9ee77c0048590d3c6e33a652eee5bc3bf522a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204440
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Turns out, even thought Heap constructs an elaborate event
object, and attaches it to the ServiceEvent, the service
does its own event construction (again).
As a result the ServiceEvent has quite a different layout
and no concept of before/after.
This CL changes Dart_GCEvent to more closely follow the
JSON event emitted by Service.
Bug: https://github.com/dart-lang/sdk/issues/43106
Change-Id: I34f33f23065ea7a92142ee7df23ab05ca91f161e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/161176
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Remove unused Dart_GlobalTimelineGetTrace and Dart_SetEmbedderTimelineCallbacks, leftover from the days of Mojo.
Change-Id: Id9ea56734155c9c0ef4f23956e0043dddd5070c0
Reviewed-on: https://dart-review.googlesource.com/c/91768
Reviewed-by: Zach Anderson <zra@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This change allows devs embedding the VM in e.g. a server to e.g. export VM metrics as part of their server health metrics.
Change-Id: I7a86a3ad98b900d30d9b7f5d19fa77f1705610c6
Reviewed-on: https://dart-review.googlesource.com/c/74723
Reviewed-by: Zach Anderson <zra@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
randomizedcoder