September 2025 - hotspot-jfr-dev

Re: RFR: 8367302: New test jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java from JDK-8366082 is failing [v9]
by David Holmes 30 Sep '25

30 Sep '25

On Tue, 23 Sep 2025 15:02:51 GMT, Johannes Bechberger <jbechberger(a)openjdk.org> wrote: >> This change hopefully fixes the test failures by making the test cases more resilient. > > Johannes Bechberger has updated the pull request incrementally with one additional commit since the last revision: > > Fix build issue Needed to add back the hotspot-jfr label as skara had removed it. JFR folk can we please get this re-reviewed and approved. Thanks. @parttimenerd perhaps you could rally other JEP 509 folk to review this fix. ------------- PR Comment: https://git.openjdk.org/jdk/pull/27293#issuecomment-3350276492

1 0

RFR: 8366896: JFR: Add gc view support for ZGC
by Erik Gahlin 25 Sep '25

25 Sep '25

Could I have a review of a PR that fixes the 'jfr gc view' when using ZGC? Could I have a review of a PR that fixes the 'jfr gc view' when using ZGC? The problem is that jdk.YoungGarbageCollection and jdk.OldGarbageCollection events are not used with ZGC. Instead, jdk.ZYoungGarbageCollection and jdk.ZOldGarbageCollection are used. Before: **$ jfr view gc recording.jfr** Garbage Collections Start GC ID Type Heap Before GC Heap After GC Longest Pause -------- ----- --------------------- -------------- ------------- ------------- 16:13:43 0 Unknown 868.0 MB 1.6 GB 0.00808 ms 16:13:43 1 Unknown 1.7 GB 3.8 GB 0.00796 ms 16:13:44 2 Unknown 3.8 GB 9.0 GB 0.0136 ms 16:13:45 3 Unknown 6.6 GB 9.0 GB 0.0203 ms 16:13:46 4 Unknown 9.0 GB 9.0 GB 0.00867 ms 16:13:48 5 Unknown 9.0 GB 9.0 GB 0.00775 ms After: **$ jfr view gc recording.jfr** Garbage Collections Start GC ID Type Heap Before GC Heap After GC Longest Pause -------- ----- ---------------------------- -------------- ------------- ------------- 16:13:43 0 ZGC Old Garbage Collection 868.0 MB 1.6 GB 0.00808 ms 16:13:43 1 ZGC Old Garbage Collection 1.7 GB 3.8 GB 0.00796 ms 16:13:44 2 ZGC Old Garbage Collection 3.8 GB 9.0 GB 0.0136 ms 16:13:45 3 ZGC Young Garbage Collection 6.6 GB 9.0 GB 0.0203 ms 16:13:46 4 ZGC Young Garbage Collection 9.0 GB 9.0 GB 0.00867 ms 16:13:48 5 ZGC Young Garbage Collection 9.0 GB 9.0 GB 0.00775 ms Note: "Unknown" can still occur if not all events for a GC ID have been emitted. I also made a change so that GCs are now ordered according to GC ID. This prevents issues where entries with a start time of N/A appear first in cases where not all events are emitted for a GC cycle. Testing: jdk/jdk/jfr + manual inspection with ZGC, G1, Parallel and Serial. Thanks Erik ------------- Commit messages: - Order by GC id - Initial Changes: https://git.openjdk.org/jdk/pull/27099/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=27099&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8366896 Stats: 5 lines in 1 file changed: 2 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/27099.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/27099/head:pull/27099 PR: https://git.openjdk.org/jdk/pull/27099

2 3

RFR: 8366809: JFR: Use factory for aggregator functions
by Erik Gahlin 23 Sep '25

23 Sep '25

Could I have a review of a PR that replaces the selection of the aggregator function with a factory, so that the selection only needs to happen once per column instead of for every row? Testing: jdk/jdk/jfr + manual inspection 'jfr view' output Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/27072/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=27072&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8366809 Stats: 40 lines in 3 files changed: 10 ins; 2 del; 28 mod Patch: https://git.openjdk.org/jdk/pull/27072.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/27072/head:pull/27072 PR: https://git.openjdk.org/jdk/pull/27072

2 2

RFR: 8367948: JFR: MethodTrace threshold setting has no effect
by Erik Gahlin 23 Sep '25

23 Sep '25

Could I have a review of the PR that ensures thresholds work with the MethodTrace event? Testing: jdk/jdk/jfr Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/27396/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=27396&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8367948 Stats: 34 lines in 3 files changed: 29 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/27396.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/27396/head:pull/27396 PR: https://git.openjdk.org/jdk/pull/27396

2 2

RFR: 8367107: JFR: Refactor policy tests out of TestRemoteDump
by Erik Gahlin 19 Sep '25

19 Sep '25

Could I get a review of a PR that splits TestRemoteDump.java into two test files? This change prevents timeouts and reduces wall clock time, as the tests can now run in parallel. As part of the change, I am reverting the modification to TestRemoteDump introduced with [JFR: Improve slow tests](https://bugs.openjdk.org/browse/JDK-8364756), which made the test flaky. I also removed unused imports and fixed the incorrect test summary. Testing: 100 * (TestRemoteDump and TestDumpRetention) Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/27153/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=27153&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8367107 Stats: 197 lines in 2 files changed: 123 ins; 73 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/27153.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/27153/head:pull/27153 PR: https://git.openjdk.org/jdk/pull/27153

2 2

Re: RFR: 8367302: New test jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java from JDK-8366082 is failing [v4]
by Johannes Bechberger 19 Sep '25

19 Sep '25

> This change hopefully fixes the test failures by making the test cases more resilient. Johannes Bechberger has updated the pull request incrementally with one additional commit since the last revision: Remove synchronized ------------- Changes: - all: https://git.openjdk.org/jdk/pull/27293/files - new: https://git.openjdk.org/jdk/pull/27293/files/5a1ec5f9..c7c72770 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=27293&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=27293&range=02-03 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/27293.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/27293/head:pull/27293 PR: https://git.openjdk.org/jdk/pull/27293

4 5

Re: RFR: 8367302: New test jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java from JDK-8366082 is failing [v3]
by Johannes Bechberger 18 Sep '25

18 Sep '25

> This change hopefully fixes the test failures by making the test cases more resilient. Johannes Bechberger has updated the pull request incrementally with one additional commit since the last revision: Tiny fixes ------------- Changes: - all: https://git.openjdk.org/jdk/pull/27293/files - new: https://git.openjdk.org/jdk/pull/27293/files/1dc6ad1d..5a1ec5f9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=27293&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=27293&range=01-02 Stats: 11 lines in 1 file changed: 0 ins; 7 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/27293.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/27293/head:pull/27293 PR: https://git.openjdk.org/jdk/pull/27293

1 0

Re: RFR: 8367302: New test jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java from JDK-8366082 is failing [v2]
by Johannes Bechberger 18 Sep '25

18 Sep '25

> This change hopefully fixes the test failures by making the test cases more resilient. Johannes Bechberger has updated the pull request incrementally with one additional commit since the last revision: Simplify code ------------- Changes: - all: https://git.openjdk.org/jdk/pull/27293/files - new: https://git.openjdk.org/jdk/pull/27293/files/15569d5d..1dc6ad1d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=27293&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=27293&range=00-01 Stats: 85 lines in 5 files changed: 18 ins; 51 del; 16 mod Patch: https://git.openjdk.org/jdk/pull/27293.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/27293/head:pull/27293 PR: https://git.openjdk.org/jdk/pull/27293

2 1

Re: RFR: 8367302: New test jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java from JDK-8366082 is failing
by Johannes Bechberger 18 Sep '25

18 Sep '25

On Mon, 15 Sep 2025 12:17:13 GMT, Johannes Bechberger <jbechberger(a)openjdk.org> wrote: > This change hopefully fixes the test failures by making the test cases more resilient. Thanks to the tips of @apangin I was able to drastically reduce both size and complexity of the test case. I hope it runs better now. What I improved - Only use a single thread - No arbitrary waits, just one native wait in the loop - Properly documented the test scenario ------------- PR Comment: https://git.openjdk.org/jdk/pull/27293#issuecomment-3307625621

1 0

Re: RFR: 8367302: New test jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java from JDK-8366082 is failing
by Andrei Pangin 18 Sep '25

18 Sep '25

On Mon, 15 Sep 2025 12:17:13 GMT, Johannes Bechberger <jbechberger(a)openjdk.org> wrote: > This change hopefully fixes the test failures by making the test cases more resilient. test/jdk/jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java line 99: > 97: } > 98: > 99: public static void main(String[] args) throws Exception { A brief high-level explanation of what the test does would be useful. >From what I understood, the test starts CPU time sampling with a short interval (1ms), temporarily disables processing of native samples to cause queue overflow, then enables it back to report lost samples. Repeats the cycle 5 times and verifies that the queue has grown by the time of the last iteration, thereby decreasing the amount of losses. test/jdk/jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java line 103: > 101: // setup recording > 102: AtomicLong burstThreadId = new AtomicLong(); > 103: final long startTimeMillis = System.currentTimeMillis(); Do not use `System.currentTimeMillis()` for time intervals: 1) it suffers from rounding errors due to low resolution; 2) it is not guaranteed to be monotonic. Use `System.nanoTime()` or `Instant.now()` instead. test/jdk/jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java line 119: > 117: rs.startAsync(); > 118: // this thread runs all along > 119: Thread burstThread = new Thread(() -> WHITE_BOX.busyWaitCPUTime(8000)); Instead of having a magic constant, is it possible to run `busyWaitCPUTime` continuously until explicitly stopped? test/jdk/jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java line 154: > 152: ThreadMXBean bean = ManagementFactory.getThreadMXBean(); > 153: long start = bean.getCurrentThreadCpuTime(); > 154: while (bean.getCurrentThreadCpuTime() - start < millis * 1_000_000) { Isn't this method supposed to check CPU time of the passed `thread` rather than current thread? test/jdk/jdk/jfr/event/profiling/TestCPUTimeSampleQueueAutoSizes.java line 165: > 163: } > 164: // check that there are at least 3 time boxes > 165: Asserts.assertTrue(timeBoxedLosses.size() > 3); Not sure what this assert means. From what I see, the size of `timeBoxedLosses` should be exactly equal to the number of main loop iterations (5). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27293#discussion_r2357301273 PR Review Comment: https://git.openjdk.org/jdk/pull/27293#discussion_r2357230947 PR Review Comment: https://git.openjdk.org/jdk/pull/27293#discussion_r2357285587 PR Review Comment: https://git.openjdk.org/jdk/pull/27293#discussion_r2357280422 PR Review Comment: https://git.openjdk.org/jdk/pull/27293#discussion_r2357246767

2 5