[S] Change in dart/sdk[main]: [vm] Allow Before/After GC to signal `perf`
0 views
Skip to first unread message
Slava Egorov (Gerrit)
Dec 15, 2025, 8:35:53 AM (6 days ago) Dec 15
to Jens Johansen, Commit Queue, rev...@dartlang.org, vm-...@dartlang.org
Attention needed from Jens Johansen
Slava Egorov voted and added 3 comments![]( "Open in Gerrit")
Votes added by Slava Egorov
| Code-Review | +1 |
3 comments
Commit Message
Line 12, Patchset 2 (Latest):
Example usage:Slava Egorov . unresolved
Cool stuff. Consider finding a place for this documentation in `runtime/docs/...` (either find an existing file this fits or add one).
File runtime/vm/heap/heap.cc
Line 1019, Patchset 2 (Latest):
#if defined(DART_HOST_OS_LINUX)Slava Egorov . unresolved
I think this belongs into `os_linux.cc`.
Line 1040, Patchset 2 (Latest):
#if defined(DART_HOST_OS_LINUX)Slava Egorov . unresolved
Move this chunk of code into `OS::Notify{Before,After}GC`
Related details
Attention is currently required from:
- Jens Johansen
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jens Johansen (Gerrit)
Dec 16, 2025, 4:11:33 AM (6 days ago) Dec 16
to Slava Egorov, Commit Queue, rev...@dartlang.org, vm-...@dartlang.org
Attention needed from Slava Egorov
Jens Johansen added 3 comments![]( "Open in Gerrit")
Commit Message
Cool stuff. Consider finding a place for this documentation in `runtime/docs/...` (either find an existing file this fits or add one).
Jens Johansen
Done
File runtime/vm/heap/heap.cc
I think this belongs into `os_linux.cc`.
Jens Johansen
Done
Move this chunk of code into `OS::Notify{Before,After}GC`
Attention is currently required from:
- Slava Egorov
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Slava Egorov (Gerrit)
Dec 16, 2025, 4:37:29 AM (6 days ago) Dec 16
to Jens Johansen, Commit Queue, rev...@dartlang.org, vm-...@dartlang.org
Attention needed from Jens Johansen
Slava Egorov voted and added 5 comments![]( "Open in Gerrit")
Votes added by Slava Egorov
| Code-Review | +1 |
5 comments
File runtime/docs/perf.md
Line 4, Patchset 3 (Latest):
`perf stat` supports starting and stopping the recording via `--control`Slava Egorov . unresolved
Empty line after `##`
(You might also consider formatting Markdown using e.g. go/mdformat - it makes it easier to read as plain text).
File runtime/vm/os_fuchsia.cc
Line 510, Patchset 3 (Latest):
void OS::NotifyBeforeGC() {}Slava Egorov . unresolved
empty line before the definition.
File runtime/vm/os_linux.cc
Line 596, Patchset 3 (Latest):
void PerfCtrlDisable() {Slava Egorov . unresolved
static or anonymous namespace (the same applies to the function below).
https://google.github.io/styleguide/cppguide.html#Internal_Linkage
File runtime/vm/os_macos.cc
Line 170, Patchset 3 (Latest):
void OS::NotifyBeforeGC() {}Slava Egorov . unresolved
add empty line before the definition.
File runtime/vm/os_win.cc
Related details
Attention is currently required from:
- Jens Johansen
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Slava Egorov (Gerrit)
Dec 16, 2025, 4:37:47 AM (6 days ago) Dec 16
to Jens Johansen, Commit Queue, rev...@dartlang.org, vm-...@dartlang.org
Attention needed from Jens Johansen
Slava Egorov added 1 comment![]( "Open in Gerrit")
Commit Message
Line 52, Patchset 3 (Latest):
Slava Egorov . unresolved
Needs `TEST=manually` to be able to land the CL.
Related details
Attention is currently required from:
- Jens Johansen
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jens Johansen (Gerrit)
Dec 16, 2025, 4:57:01 AM (6 days ago) Dec 16
to Slava Egorov, Commit Queue, rev...@dartlang.org, vm-...@dartlang.org
Jens Johansen added 7 comments![]( "Open in Gerrit")
Patchset-level comments
Commit Message
Needs `TEST=manually` to be able to land the CL.
Jens Johansen
Done
File runtime/docs/perf.md
Line 4, Patchset 3:
`perf stat` supports starting and stopping the recording via `--control`Slava Egorov . resolved
Empty line after `##`
(You might also consider formatting Markdown using e.g. go/mdformat - it makes it easier to read as plain text).
Jens Johansen
Done
File runtime/vm/os_fuchsia.cc
empty line before the definition.
Jens Johansen
Done
File runtime/vm/os_linux.cc
static or anonymous namespace (the same applies to the function below).
https://google.github.io/styleguide/cppguide.html#Internal_Linkage
Jens Johansen
Done
File runtime/vm/os_macos.cc
add empty line before the definition.
Jens Johansen
Done
File runtime/vm/os_win.cc
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jens Johansen (Gerrit)
Dec 16, 2025, 6:50:35 AM (5 days ago) Dec 16
to Slava Egorov, Commit Queue, rev...@dartlang.org, vm-...@dartlang.org
Jens Johansen voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Commit Queue (Gerrit)
Dec 16, 2025, 7:16:20 AM (5 days ago) Dec 16
to Jens Johansen, Slava Egorov, rev...@dartlang.org, vm-...@dartlang.org
Commit Queue submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: runtime/vm/os_fuchsia.cc
Insertions: 1, Deletions: 0.
@@ -507,6 +507,7 @@
void OS::DebugBreak() {
UNIMPLEMENTED();
}
+
void OS::NotifyBeforeGC() {}
void OS::NotifyAfterGC() {}
```
```
The name of the file: runtime/docs/perf.md
Insertions: 13, Deletions: 9.
@@ -1,12 +1,16 @@
# Dart support for perf
## Pause `perf stat` recording while GC is running (or vice versa)
-`perf stat` supports starting and stopping the recording via `--control`
-and the Dart VM can be instructed to either stop the recording while
-GC is running, or specifically start while GC is running (and stop when GC is finished)
-with `--perf_ctl_fd`, `--perf_ctl_fd_ack` (for file handles) and `--perf_ctl_usage` (1 or 2) to specify if it should be paused while GCing (1) or started (and stoped) while GCing (2).
-These options can for instance be useful for testing performance changes in AOT compiled dart together with `--deterministic`.
+`perf stat` supports starting and stopping the recording via `--control` and the
+Dart VM can be instructed to either stop the recording while GC is running, or
+specifically start while GC is running (and stop when GC is finished) with
+`--perf_ctl_fd`, `--perf_ctl_fd_ack` (for file handles) and `--perf_ctl_usage`
+(1 or 2) to specify if it should be paused while GCing (1) or started (and
+stoped) while GCing (2).
+
+These options can for instance be useful for testing performance changes in AOT
+compiled dart together with `--deterministic`.
As an example one could run a script like this:
@@ -25,7 +29,7 @@
(the missing pieces can be found in the perf stat man page).
-This will start `perf stat` paused (which would require the run dart aot-compiled script to start it when it wants to) and where the VM pauses `perf` while doing GC.
-Note that the recording will be started when GC is done, even if it wasn't started before.
-
-
+This will start `perf stat` paused (which would require the run dart
+aot-compiled script to start it when it wants to) and where the VM pauses `perf`
+while doing GC. Note that the recording will be started when GC is done, even if
+it wasn't started before.
```
```
The name of the file: runtime/vm/os_linux.cc
Insertions: 2, Deletions: 0.
@@ -593,6 +593,7 @@
__builtin_trap();
}
+namespace {
void PerfCtrlDisable() {
if (FLAG_perf_ctl_fd > 0 && FLAG_perf_ctl_fd_ack > 0) {
write(FLAG_perf_ctl_fd, "disable", 7);
@@ -610,6 +611,7 @@
read(FLAG_perf_ctl_fd_ack, ack, 5);
}
}
+} // namespace
void OS::NotifyBeforeGC() {
if (FLAG_perf_ctl_usage == 1) {
```
```
The name of the file: runtime/vm/os_win.cc
Insertions: 1, Deletions: 0.
@@ -280,6 +280,7 @@
}
#endif
}
+
void OS::NotifyBeforeGC() {}
void OS::NotifyAfterGC() {}
```
```
The name of the file: runtime/vm/os_macos.cc
Insertions: 1, Deletions: 0.
@@ -167,6 +167,7 @@
void OS::DebugBreak() {
__builtin_trap();
}
+
void OS::NotifyBeforeGC() {}
void OS::NotifyAfterGC() {}
```
Change information
Commit message:
[vm] Allow Before/After GC to signal `perf`
This CL makes it possible for the Dart VM to start or stop `perf` while
doing GC, which gives a significantly more stable output.
Example usage:
Bash script that creates the correct file handles and calls dart under
`perf stat` based on the arguments given to the script:
```
DART=$1
shift
AOTSNAPSHOT=$1
shift
ctl_dir=/tmp/
ctl_fifo=${ctl_dir}perf_ctl.fifo
test -p ${ctl_fifo} && unlink ${ctl_fifo}
mkfifo ${ctl_fifo}
exec {ctl_fd}<>${ctl_fifo}
ctl_ack_fifo=${ctl_dir}perf_ctl_ack.fifo
test -p ${ctl_ack_fifo} && unlink ${ctl_ack_fifo}
mkfifo ${ctl_ack_fifo}
exec {ctl_fd_ack}<>${ctl_ack_fifo}
perf_ctl_fd=$ctl_fd perf_ctl_fd_ack=$ctl_fd_ack perf stat --delay=-1 --control fd:${ctl_fd},${ctl_fd_ack} -B -e "task-clock:u,context-switches:u,cpu-migrations:u,page-faults:u,cycles:u,instructions:u,branch-misses:u" $DART --perf_ctl_fd=${ctl_fd} --perf_ctl_fd_ack=${ctl_fd_ack} --perf_ctl_usage=1 --deterministic $AOTSNAPSHOT $@
exec {ctl_fd_ack}>&-
unlink ${ctl_ack_fifo}
exec {ctl_fd}>&-
unlink ${ctl_fifo}
```
This will start `perf stat` paused (which would require the run dart
aot-compiled script to start it when it wants to) and where the VM
pauses `perf` while doing GC.
In practise, looking at instruction counts reported by `perf stat`, I've
seen the difference between runs go from 2+ mio (and in some instances
23+ mio) to around 30,000 (!) on runs of the analyzer (tool
"stable_analysis").
TEST=manually
Change-Id: Iab955a5dd35e47f22c4f693ae50effc8ee633897
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/468260
Reviewed-by: Slava Egorov <veg...@google.com>
Commit-Queue: Jens Johansen <je...@google.com>
Files:
- A runtime/docs/perf.md
- M runtime/vm/flag_list.h
- M runtime/vm/heap/heap.cc
- M runtime/vm/os.h
- M runtime/vm/os_android.cc
- M runtime/vm/os_fuchsia.cc
- M runtime/vm/os_linux.cc
- M runtime/vm/os_macos.cc
- M runtime/vm/os_win.cc
Change size: M
Delta: 9 files changed, 106 insertions(+), 0 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Slava Egorov
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ryan Macnak (Gerrit)
Dec 17, 2025, 2:07:22 PM (4 days ago) Dec 17
to Jens Johansen, Commit Queue, Slava Egorov, rev...@dartlang.org, vm-...@dartlang.org
Ryan Macnak added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 6 (Latest):
Ryan Macnak . resolved
significantly more stable output.
--deterministic
You may want to also use --marker_tasks=1.
The --deterministic flag was added with the goal of making snapshot output deterministic. It disables concurrent marking and sweeping to avoid affecting what is seen by full walks of the heap. But it does not disable parallel marking because that doesn't affect which objects survive a given GC cycle and so doesn't affect snapshot output, though it will affect perf determinism as the workers race to handle the marking worklists.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jens Johansen (Gerrit)
Dec 18, 2025, 3:09:23 AM (4 days ago) Dec 18
to Commit Queue, Ryan Macnak, Slava Egorov, rev...@dartlang.org, vm-...@dartlang.org
Jens Johansen added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Ryan Macnak . resolved
significantly more stable output.
--deterministicYou may want to also use --marker_tasks=1.
The --deterministic flag was added with the goal of making snapshot output deterministic. It disables concurrent marking and sweeping to avoid affecting what is seen by full walks of the heap. But it does not disable parallel marking because that doesn't affect which objects survive a given GC cycle and so doesn't affect snapshot output, though it will affect perf determinism as the workers race to handle the marking worklists.
Jens Johansen
Interesting, thanks!
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages