August 2024 - hotspot-jfr-dev

RFR: 8279016: JFR Leak Profiler is broken with Shenandoah
by Aleksey Shipilev 06 Nov '24

06 Nov '24

While testing unrelated Shenandoah patch, I caught a GC assert when Leak Profiler was running ([JDK-8337194](https://bugs.openjdk.org/browse/JDK-8337194)) Leak Profiler is notorious in using the mark words for its own needs. We have been trying to mitigate its impact on GCs by moving to separate bitsets for tracking marked objects, or by treating "marked without fwdptr" as "JFR marked" and handling it. But this is not reliable, since things like [putting indexes in mark word](https://github.com/openjdk/jdk/blob/3baff2af6a30cc6cb2e0d4391db1cf7e6… sneak in. This is okay for Leak Profiler alone, since it restores the mark words after the operation completes, but that is still not enough when GC is already running. I say we side-step this whack-a-mole by cleanly bailing from JFR op, when we know it is unsafe to do. I thought to use `VM_Operation::doit_prologue`, but I think GC start may sneak in between checking in prologue and op start. This realistically only affects Shenandoah. All other STW collectors would never see what Leak Profiler did with mark words. ZGC would not see it, since it does not care about mark words for its own operation. Additional testing: - [x] `jdk/jfr/event/oldobject/` pass by default (100x times) - [x] `jdk/jfr/event/oldobject/` pass with `-XX:+UseShenandoah` (100x) ------------- Commit messages: - Fix Changes: https://git.openjdk.org/jdk/pull/20328/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20328&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8279016 Stats: 29 lines in 4 files changed: 29 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/20328.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20328/head:pull/20328 PR: https://git.openjdk.org/jdk/pull/20328

7 46

RFR: 8339191: JFR ChunkInputStream supports bulk read
by David Schlosnagle 30 Aug '24

30 Aug '24

I would like to contribute an improvement to reading JFR recordings via `InputStream` by implementing `InputStream#read(byte[], int, int)` in `ChunkInputStream` to support bulk reads and avoid many expensive single byte `read()`. Testing: `test/jdk/jdk/jfr/api/consumer/TestChunkInputStreamBulkRead.java`, `jdk_jfr` While looking through some JFRs recently, I noticed some significant time spent in `java.io.BufferedInputStream#getBufIfOpen()` that traced back to transferring (via `InputStream#transferTo(OutputStream)` JFR profile recording being read via `InputStream` returned by `jdk.jfr.Recording#getStream(Instant , Instant)`. What caught my eye were calls to single byte `jdk.jfr.internal.ChunkInputStream#read()` rather than `read(byte[], int, int)` as seen in the stacktrace from JFR. <img width="776" alt="image" src="https://github.com/user-attachments/assets/b92398cd-2761-4600-8cfe-ea1bada6…"> ------------- Commit messages: - JFR ChunkInputStream supports bulk read Changes: https://git.openjdk.org/jdk/pull/20534/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20534&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8339191 Stats: 123 lines in 2 files changed: 116 ins; 3 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/20534.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20534/head:pull/20534 PR: https://git.openjdk.org/jdk/pull/20534

3 10

RFR: 8339214: Remove misleading CodeBuilder.loadConstant(Opcode, ConstantDesc)
by Chen Liang 29 Aug '24

29 Aug '24

8339214: Remove misleading CodeBuilder.loadConstant(Opcode, ConstantDesc) ------------- Commit messages: - Improve specs for ConstantInstruction.ofArgument - 8339214: Remove misleading CodeBuilder.loadConstant(Opcode, ConstantDesc) Changes: https://git.openjdk.org/jdk/pull/20779/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20779&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8339214 Stats: 205 lines in 10 files changed: 7 ins; 157 del; 41 mod Patch: https://git.openjdk.org/jdk/pull/20779.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20779/head:pull/20779 PR: https://git.openjdk.org/jdk/pull/20779

1 0

RFR: 8339149: jfr_flush_event_writer - return value type mismatch
by Markus Grönlund 28 Aug '24

28 Aug '24

Very small adjustment to the function signature. Testing: jdk_jfr Thanks Markus ------------- Commit messages: - 8339149 Changes: https://git.openjdk.org/jdk/pull/20742/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20742&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8339149 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/20742.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20742/head:pull/20742 PR: https://git.openjdk.org/jdk/pull/20742

2 2

RFR: 8338939: Simplify processing of hidden class names
by Markus Grönlund 27 Aug '24

27 Aug '24

Greetings, The name symbol for hidden (anonymous) classes has evolved. Initially, it consisted of only a base class name, e.g., "java/lang/invoke/LambdaForm$BMH", so JFR had to introduce a scheme for disambiguating one anonymous class from another. The scheme (which still exists in JFR) appends the Java class mirror's hashcode to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH/22626602". With [JDK-8238358](https://bugs.openjdk.org/browse/JDK-8238358), the disambiguation of hidden class names is now intrinsic to the processing by the ClassFileParser. This is done by appending the InstanceKlass address to the base class name, e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000". An argument can be made that JFR specialization for hidden (anonymous) classes may no longer be necessary. Indeed, the result now involves a "double" disambiguation: e.g., "java/lang/invoke/LambdaForm$BMH+0x000000001d190000/231202600". It is now possible to simplify the handling of hidden (anonymous) class names, which avoids using the Java mirror oop. We might want to keep the "/" construct as that might be an implicit invariant on users' expectations (quickly done by replacing "+" with "/"). Testing: jdk_jfr Thanks Markus ------------- Commit messages: - external_name - 8338939 Changes: https://git.openjdk.org/jdk/pull/20708/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20708&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8338939 Stats: 58 lines in 2 files changed: 2 ins; 17 del; 39 mod Patch: https://git.openjdk.org/jdk/pull/20708.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20708/head:pull/20708 PR: https://git.openjdk.org/jdk/pull/20708

3 5

RFR: 8338389: [JFR] Long strings should be added to the string pool
by Joakim Nordström 27 Aug '24

27 Aug '24

When committing JFR events with string data longer than 128 characters, the string isn't added to the JFR string pool. In scenarios with many events containing large string values, this can lead to very large JFR recordings. This is mostly the case for custom events containing f.i. SQL strings or other data. In the case where the string data consist of mostly duplicate data, adding them to the string pool has been shown to reduce the recording size by a factor of 10. This change will add all longer strings (>128 characters) to the string pool. Strings of length 16-128 are still added using the same pre-cache methodology as before. The raised StringPool.MAX_LIMIT is used by [`jdk.jfr.internal.event.EventWriter::putString`](https://github.com/openjdk/jdk/blob/master/src/jdk.jfr/share/classes/jdk/jfr/internal/event/EventWriter.java) to determine which strings to add to the pool. ## Testing - tier1-tier3 - jdk_jfr - Internal testing with high load, using custom JFR events containing long strings have been performed ------------- Commit messages: - 8338389: [JFR] Long strings should be added to the string pool - 8338389: [JFR] Long strings should be added to the string pool Changes: https://git.openjdk.org/jdk/pull/20596/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20596&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8338389 Stats: 122 lines in 2 files changed: 118 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/20596.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20596/head:pull/20596 PR: https://git.openjdk.org/jdk/pull/20596

4 9

RFR: 8338938: The result of the combine method of SettingsControl is not used
by Chihiro Ito 27 Aug '24

27 Aug '24

Could you please review this PR that SettingsControl does not work properly when configured with multiple records. Regards, Chihiro ------------- Commit messages: - Fixed to use the return value of the combine method of SettingsControl Changes: https://git.openjdk.org/jdk/pull/20707/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20707&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8338938 Stats: 31 lines in 2 files changed: 29 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/20707.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20707/head:pull/20707 PR: https://git.openjdk.org/jdk/pull/20707

2 2

RFR: 8338819: JFR: Internal events causes crash when no other events are in use
by Erik Gahlin 23 Aug '24

23 Aug '24

Could I have a review of a PR that fixes an issue with internal events. Testing: tier1 + jdk/jdk/jfr + manual check Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/20676/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20676&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8338819 Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/20676.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20676/head:pull/20676 PR: https://git.openjdk.org/jdk/pull/20676

3 3

RFR: 8338417: Explicitly pin a virtual thread before acquiring the JFR string pool monitor
by Markus Grönlund 23 Aug '24

23 Aug '24

Greetings, Explicitly pin a virtual thread before acquiring the JFR string pool monitor because migrating a carrier thread local event writer object onto another carrier thread is impossible. During event commit, the thread is in a critical section because it has loaded a carrier thread local event writer object. For virtual threads, a contended monitor, such as a synchronized block, is a point where a thread could become unmounted. A monitor guards the JFR string pool, but remounting a virtual thread onto another carrier is impossible because of the event writer. Therefore, it's imperative to use explicit pin constructs to prevent unmounting at this location. Testing: jdk_jfr Thanks Markus ------------- Commit messages: - 8338417 Changes: https://git.openjdk.org/jdk/pull/20588/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20588&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8338417 Stats: 173 lines in 3 files changed: 149 ins; 8 del; 16 mod Patch: https://git.openjdk.org/jdk/pull/20588.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20588/head:pull/20588 PR: https://git.openjdk.org/jdk/pull/20588

4 18

RFR: 8338512: JFR: Revert changes to TestCodeSweeper
by Erik Gahlin 19 Aug '24

19 Aug '24

Could I have a review of a PR that reverts the change to TestCodeSweeper.java. Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/20617/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20617&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8338512 Stats: 41 lines in 2 files changed: 3 ins; 24 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/20617.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20617/head:pull/20617 PR: https://git.openjdk.org/jdk/pull/20617

2 2