July 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

RFR: 8309871: jdk/jfr/api/consumer/recordingstream/TestSetEndTime.java timed out
by Erik Gahlin 03 Dec '23

03 Dec '23

Could I have review of a fix that adds -XX:-UseFastUnorderedTimeStamps to a test. An argument can be made that we should test features same way as they are used in the product, and I generally agree, but it's hard to fix this issue without a performance impact. Also, I'm not 100% certain that different clock sources are the underlying problem. By running with -XX:-UseFastUnorderedTimeStamps we will gain additional information on this intermittent test failure. Putting the test on the problem list might hide other issue. Testing: jdk/jdk/jfr Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/14975/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14975&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8309871 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/14975.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14975/head:pull/14975 PR: https://git.openjdk.org/jdk/pull/14975

3 3

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: 8313394: Array Elements in OldObjectSample event should be -1 if the object is not an array
by KIRIYAMA Takuya 04 Aug '23

04 Aug '23

With this fix, arrayElements attribute in OldObjectSample event will be -1 if the object is not an array. Could anyone please review this fix? ------------- Commit messages: - 8313394: Array Elements in OldObjectSample event should be -1 if the object is not an array - 8313394: Array Elements in OldObjectSample event should be -1 if the object is not an array Changes: https://git.openjdk.org/jdk/pull/15084/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15084&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8313394 Stats: 108 lines in 2 files changed: 107 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15084.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15084/head:pull/15084 PR: https://git.openjdk.org/jdk/pull/15084

2 9

Re: RFR: JDK-8313251: Add NativeLibraryLoad event [v3]
by Matthias Baesken 02 Aug '23

02 Aug '23

> Add a NativeLibraryLoad event that provides us more detail about shared lib/dll loads. This gives a time stamp and success + error details of the load operation. It enhances the already existing information we get from the existing NativeLibrary event (that periodically samples the native modules of the jvm process). Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust COPYRIGHT year info ------------- Changes: - all: https://git.openjdk.org/jdk/pull/15065/files - new: https://git.openjdk.org/jdk/pull/15065/files/a934d3f7..da6b9861 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15065&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15065&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15065.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15065/head:pull/15065 PR: https://git.openjdk.org/jdk/pull/15065

3 5

Re: RFR: 8313141: Missing check for os_thread type in os_windows.cpp [v2]
by Julian Waters 01 Aug '23

01 Aug '23

> os::os_thread is not handled in os_windows.cpp, and therefore ignores the thread stack size option. I am unsure if this is the right way to handle os::os_thread, please let me know if it is not Julian Waters has updated the pull request incrementally with one additional commit since the last revision: Switch os::os_thread to default behaviour ------------- Changes: - all: https://git.openjdk.org/jdk/pull/15079/files - new: https://git.openjdk.org/jdk/pull/15079/files/9c136e64..bf3cf8ea Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15079&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15079&range=00-01 Stats: 3 lines in 1 file changed: 2 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/15079.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15079/head:pull/15079 PR: https://git.openjdk.org/jdk/pull/15079

3 3