June 2023 - hotspot-jfr-dev

RFR: 8276125: RunThese24H.java SIGSEGV in JfrThreadGroup::thread_group_id
by Markus Grönlund 04 Jun '24

04 Jun '24

Greetings, please help review this changeset. Details are listed in the the JIRA issue. Testing: jdk_jfr Thanks Markus ------------- Commit messages: - 8276125 Changes: https://git.openjdk.java.net/jdk/pull/6702/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6702&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276125 Stats: 6 lines in 1 file changed: 4 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6702.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6702/head:pull/6702 PR: https://git.openjdk.java.net/jdk/pull/6702

5 6

RFR: 8294978: Convert 5 test/jdk/jdk/jfr tests from ASM library to Classfile API
by Chen Liang 03 May '24

03 May '24

Converted these tests to Classfile API and some of the constants to Constant API. InstrumentationCallback.callback wasn't changed to a record since I cannot figure out how to ship a nested record class with InstrumentationCallback in a jar. ------------- Commit messages: - Convert jdk.jfr tests to ASM Changes: https://git.openjdk.org/jdk/pull/13072/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=13072&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8294978 Stats: 283 lines in 5 files changed: 75 ins; 108 del; 100 mod Patch: https://git.openjdk.org/jdk/pull/13072.diff Fetch: git fetch https://git.openjdk.org/jdk pull/13072/head:pull/13072 PR: https://git.openjdk.org/jdk/pull/13072

3 19

Priority redistribution in Old Object Sample event
by Galder Zamarreno 24 Jan '24

24 Jan '24

Hi, I've been porting over JFR Old Object Sample event to GraalVM and I had the following doubt about the code in HotSpot: I'm trying to understand the code in [1]: ``` // push span onto previous if (previous != NULL) { _priority_queue->remove(previous); previous->add_span(sample->span()); _priority_queue->push(previous); } ``` This code is related to old object sampling. When the queue is full, samples that have been GC'd are removed from the queue. To do so it takes its previous (from the list view) and adds the span of the removed dead object onto it. Basically, I'm wondering why this is done this way. My initial gut feeling when I first saw the code is that it does it so that the priority queue does not get too imbalanced (it uses a heap underneath) when removing objects, but that only makes sense to me if the previous is not the previous object sampled based on time, but the previous object sampled based on the order of the priority queue, so that's not what seems to happen. I'm wondering now if the fact that the list view is ordered by insertion time is relevant here. Is it maybe trying to give more importance (by increasing its span) to surviving objects that are near in time to those that have been garbage collected? Something that's probably related to this is how the span is initially calculated. This is done in [2], which you can find below: ``` _total_allocated += allocated; const size_t span = _total_allocated - _priority_queue->total(); ``` Again when I first looked at it, I thought: well that's just equivalent to: ``` const size_t span = allocated; ``` But clearly things are more nuanced than that. The only documentation I've found so far is in Marcus' blog post [3]: > If sampled objects are garbage collected, they are removed and the priority redistributed to the neighbours. The priority is called span, and is currently the size of the allocation, giving more weight to larger (therefore more severe) leaks. Unfortunately what is a neighbour is not explained, nor why the priority gets redistributed. Thanks Galder [1] https://github.com/openjdk/jdk17u-dev/blob/master/src/hotspot/share/jfr/lea… [2] https://github.com/openjdk/jdk17u-dev/blob/ef86ea2842b1a204834291d9d6665bfc… [3] https://hirt.se/blog/?p=1055

4 7

Re: JFR integration
by Alan Bateman 17 Oct '23

17 Oct '23

On 22/03/2023 17:23, Jonathan Ross wrote: > Hi Alan, and thanks for the response. > > I would agree that naming virtual threads, particularly when using a > VirtualThreadPerTaskExecutor, would not be that useful. No, what I'm > actually asking is whether we could still fill in useful carrier > thread information in the JFR events, and whether we could name the > carrier threads. (I'm using the right term, I hope? The platform > threads on which the virtual threads are run?) > > Let me try to clarify the nameless thread bit. If I look at a flight > recording running a large number of jobs using a > VirtualThreadPerTaskExecutor, then the custom events I record that get > fired on virtual threads don't have any thread information. If I look > at the JMC threadview, I see all my custom events on a single row > without a thread name. If I browse the event list, I also don't see > any thread information. > > I just tried looking at the raw data I captured using the jfr tool > shipped with openjdk 20, but it fails: > > jfr print: unexpected internal error, Pool jdk.types.StackTrace must > contain at least one element > java.lang.InternalError: Pool jdk.types.StackTrace must contain at > least one element > at > jdk.jfr/jdk.jfr.internal.consumer.ChunkParser.fillConstantPools(ChunkParser.java:347) > at > jdk.jfr/jdk.jfr.internal.consumer.ChunkParser.<init>(ChunkParser.java:149) > at > jdk.jfr/jdk.jfr.internal.consumer.ChunkParser.nextChunkParser(ChunkParser.java:158) > at jdk.jfr/jdk.jfr.consumer.RecordingFile.findNext(RecordingFile.java:252) > at > jdk.jfr/jdk.jfr.consumer.RecordingFile.readEvent(RecordingFile.java:111) > at > jdk.jfr/jdk.jfr.internal.tool.EventPrintWriter.print(EventPrintWriter.java:75) > at jdk.jfr/jdk.jfr.internal.tool.Print.execute(Print.java:163) > at jdk.jfr/jdk.jfr.internal.tool.Main.main(Main.java:87) > > It looks like JMC isn't the only tool that gets confused by virtual > threads in flight recordings. > I'm not aware of any issues or if the parsing exception in your mail is related to virtual threads or not, maybe hotspot-jfr-dev may recognize this. As a quick test, I tried some custom events committed from virtual threads and the jfr tool parses the recording, e.g. jfr print --events *Foo* ... Foo { startTime = 17:50:05.595 (2023-03-22) duration = 0.00147 ms message = "A message from a virtual thread" eventThread = "" (javaThreadId = 35, virtual) stackTrace = [ Test.lambda$main$0() line: 21 java.util.concurrent.FutureTask.run() line: 317 java.lang.VirtualThread.run(Runnable) line: 314 ] } Note the eventThread line has the thread name (empty in this case), the thread ID, and it also indicates that it is a virtual thread. -Alan

3 4

RFR: 8308995: Update Network IO JFR events to be static mirror events
by Tim Prinzing 22 Sep '23

22 Sep '23

The socket read/write JFR events currently use instrumentation of java.base code using templates in the jdk.jfr modules. This results in some java.base code residing in the jdk.jfr module which is undesirable. JDK19 added static support for event classes. The old instrumentor classes should be replaced with mirror events using the static support. In the java.base module: Added two new events, jdk.internal.event.SocketReadEvent and jdk.internal.event.SocketWriteEvent. java.net.Socket and sun.nio.ch.SocketChannelImpl were changed to make use of the new events. In the jdk.jfr module: jdk.jfr.events.SocketReadEvent and jdk.jfr.events.SocketWriteEvent were changed to be mirror events. In the package jdk.jfr.internal.instrument, the classes SocketChannelImplInstrumentor, SocketInputStreamInstrumentor, and SocketOutputStreamInstrumentor were removed. The JDKEvents class was updated to reflect all of those changes. The existing tests in test/jdk/jdk/jfr/event/io continue to pass with the new implementation: Passed: jdk/jfr/event/io/TestSocketChannelEvents.java Passed: jdk/jfr/event/io/TestSocketEvents.java I added a micro benchmark which measures the overhead of handling the jfr socket events. test/micro/org/openjdk/bench/java/net/SocketEventOverhead.java. It needs access the jdk.internal.event package, which is done at runtime with annotations that add the extra arguments. At compile time the build arguments had to be augmented in make/test/BuildMicrobenchmark.gmk ------------- Commit messages: - some changes from review. - fix copyright date - Added micro benchmark to measure socket event overhead. - Some changes from review. - remove unnecessary cast - 8308995: Update Network IO JFR events to be static mirror events Changes: https://git.openjdk.org/jdk/pull/14342/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14342&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8308995 Stats: 896 lines in 12 files changed: 523 ins; 367 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/14342.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14342/head:pull/14342 PR: https://git.openjdk.org/jdk/pull/14342

5 37

RFR: 8307057 JFR: Enable -XX:+DebugNonSafepoints when JFR starts
by Erik Gahlin 16 Sep '23

16 Sep '23

Could I have review of a change that enables DebugNonSafepoints when JFR starts. Testing: tier1, tier2, tier3, tier4 Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/14147/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14147&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8307057 Stats: 9 lines in 2 files changed: 9 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14147.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14147/head:pull/14147 PR: https://git.openjdk.org/jdk/pull/14147

4 4

RFR: 8307526: [JFR] Better handling of tampered JFR repository
by Joakim Nordström 05 Jul '23

05 Jul '23

This change makes sure that "JFR Periodic Task" isn't interrupted by any errors. This can happen when an unfinished chunkfile is removed "in-flight", which would lead to the chunkfiles not being rotated properly, and the maxsize/maxage being ignored. With this fix, when an unfinished chunkfile is detected, all of the chunkfiles in the recording are checked for existence (since one likely cause for this could be f.i. 'rm -r /tmp' being invoked, effectively deleting all chunkfiles). Upon seeing missing chunkfiles, an error is logged, and emitted to the recording to signal that some data might be missing. A check to catch any missing chunkfiles is also added when dumping a recording -- this can happen if only finished chunkfiles have been removed, which is not detected in-flight. Also, a check was added to the file purger to check whether the file had already been deleted to not fill the purge list with already removed files. ------------- Commit messages: - 8307526: [JFR] Better handling of tampered JFR repository - 8307526: [JFR] Better handling of tampered JFR repository - 8307526: [JFR] Better handling of tampered JFR repository - 8307526: [JFR] Better handling of tampered JFR repository - 8307526: [JFR] Better handling of tampered JFR repository Changes: https://git.openjdk.org/jdk/pull/14360/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14360&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8307526 Stats: 172 lines in 5 files changed: 137 ins; 11 del; 24 mod Patch: https://git.openjdk.org/jdk/pull/14360.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14360/head:pull/14360 PR: https://git.openjdk.org/jdk/pull/14360

3 21

RFR: 8310661: JFR: Replace JVM.getJVM() with JVM
by Erik Gahlin 03 Jul '23

03 Jul '23

Could I have a review of a change that makes instance methods in the JVM class static. Today some methods are static, others are not, which means the JVM object needs to be stored and passed around. A singleton object may have made sense in JDK 8 so JVM::getJVM() could be made caller sensitive, similar to Unsafe::getUnsafe(), but not anymore. The jdk.jfr.internal package is protected by the module system. Testing: jdk/jdk/jfr tier1 + tier2 Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/14713/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14713&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8310661 Stats: 406 lines in 36 files changed: 25 ins; 72 del; 309 mod Patch: https://git.openjdk.org/jdk/pull/14713.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14713/head:pull/14713 PR: https://git.openjdk.org/jdk/pull/14713

2 2

[jdk21] RFR: 8311007: jdk/jfr/tool/TestView.java can't find event
by Erik Gahlin 29 Jun '23

29 Jun '23

8311007: jdk/jfr/tool/TestView.java can't find event ------------- Commit messages: - Backport e3f18af1dfca1be292a76ed209a7e4ca75ca94da Changes: https://git.openjdk.org/jdk21/pull/79/files Webrev: https://webrevs.openjdk.org/?repo=jdk21&pr=79&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8311007 Stats: 25 lines in 1 file changed: 25 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk21/pull/79.diff Fetch: git fetch https://git.openjdk.org/jdk21.git pull/79/head:pull/79 PR: https://git.openjdk.org/jdk21/pull/79

2 2

RFR: 8311007: jdk/jfr/tool/TestView.java can't find event
by Erik Gahlin 28 Jun '23

28 Jun '23

Could I have a review of a test fix that ensures that the event timestamp are in sync. with the timestamp used by 'jcmd view'. Testing: test/jdk/jdk/jfr/jcmd/TestJcmdView.java Thanks Erik ------------- Commit messages: - Remove unused import - Typo - Initial Changes: https://git.openjdk.org/jdk/pull/14686/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14686&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8311007 Stats: 25 lines in 1 file changed: 25 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/14686.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14686/head:pull/14686 PR: https://git.openjdk.org/jdk/pull/14686

2 2