May 2024 - hotspot-jfr-dev

RFR: 8310994: Add JFR event for selection operations
by Tim Prinzing 25 Jun '24

25 Jun '24

Added mirror event with static methods: jdk.internal.event.SelectionEvent that provides the duration of select calls and the count of how many keys are available. Emit the event from SelectorImpl::lockAndDoSelect Test at jdk.jfr.event.io.TestSelectionEvents ------------- Commit messages: - remove trailing whitespace - event logic outside of the lock, selector in try block - remove unused import - fix TestConfigure failure - add event defaults - Merge branch 'master' into JDK-8310994 - minor test cleanup - Merge branch 'master' into JDK-8310994 - 8310994: Add JFR event for selection operations Changes: https://git.openjdk.org/jdk/pull/16710/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16710&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8310994 Stats: 265 lines in 9 files changed: 264 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16710.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16710/head:pull/16710 PR: https://git.openjdk.org/jdk/pull/16710

6 40

Priority redistribution in Old Object Sample event
by Stig Rohde Døssing 04 Jun '24

04 Jun '24

Hi, I'm having a hard time understanding the priority redistribution code for OldObjectSample. Some months ago, there was a thread about it at https://mail.openjdk.org/pipermail/hotspot-jfr-dev/2023-June/005066.html, but it didn't conclude one way or the other whether this code looked correct. I'm hoping to understand the intent, and maybe also discuss whether the current implementation follows that intent. I'll be describing my understanding in order to surface any misunderstandings, so apologies for the incoming wall of text. Please correct me if I'm getting things wrong. # The intent If we think of allocations as a "timeline", where each sampled object extends the timeline by adding more bytes to the right side of the line, my understanding is that the goal of this code is to ensure that the samples we keep around in the priority list are as evenly distributed across this timeline as possible. We'd presumably like a good distribution whether objects are garbage collected or not. Going by the earlier mailing list comment at https://mail.openjdk.org/pipermail/hotspot-jfr-dev/2024-January/005847.html, it seems like the intent was that the sample spans should cover the entire allocation timeline when summed. I think the invariant that's being aimed for is that a sample always covers the span of bytes allocated since the older neighboring sample was taken. When a node is removed, the span of the removed node should be given to the younger neighbor, which ensures the neighbor will cover the span left missing by the node being removed. When a node is removed that has no younger neighbor, the span should be given to the next sample added to the queue, which ensures the new sample will cover the span to the older neighbor. The total spans of the nodes in the queue will then nearly sum to the total allocated bytes, and any missing bit (due to removal of the youngest sample/rejection of incoming samples because their spans are too small) will be covered by the next sample added to the queue. I think the idea is that by continually adjusting sample spans as neighbors are GC'ed or otherwise removed, and by keeping the samples with the largest spans, we'll end up with samples that are well distributed across the timeline. # The current implementation When an object is added, the sampler calculates a new span by counting how many bytes have been sampled so far (including the sample potentially being added), and subtracting the spans of the items already in the queue. https://github.com/openjdk/jdk/blob/32c7681cf310c87669c502c4a8b62a7fecc9336… . This calculated span is used only for deciding whether to add the new sample to the queue. If added, the sample gets a span equal to the byte size of the object being allocated. https://github.com/openjdk/jdk/blob/32c7681cf310c87669c502c4a8b62a7fecc9336… When a sample is dropped due to GC, the span it covers is handed to the `prev` sample in the list. As far as I can tell, the list is ordered youngest-sample-first, so this adds the span to the younger neighbor sample. If there is no younger neighbor, the span is discarded. https://github.com/openjdk/jdk/blob/32c7681cf310c87669c502c4a8b62a7fecc9336… When a sample is dropped in favor of a new sample with a larger span, the old sample's span is discarded. https://github.com/openjdk/jdk/blob/32c7681cf310c87669c502c4a8b62a7fecc9336… # The bits I think might not fit the intent * The span stored in each sample is not the calculated span, it's just the object's byte size (`allocated`). That means as soon as any object falls out of the queue, the spans in the queue no longer sum to cover the allocation timeline. This causes all future samples to be added to be unduly prioritized for adding to the queue, because they are given an artificially high span. In effect, future samples are weighted as if they cover both the interval between themselves and the older neighbor sample, plus all "missing spans" from nodes that have been discarded since the program started. Just to demonstrate that there might be a problem, I tried limiting the sampler queue size to 3, and calling the `ObjectSampler::add` method 10 times in a row with a dummy object and a fixed byte size of 10. The result is that the 1st, 2nd and 10th allocations are kept. This is because after the first 3 inserts to fill the queue, the 4th sample is discarded because the calculated span is 40 - 30 = 10. The 5th item will calculate the span 50 - 30 = 20, and so replace the smallest sample (in span terms), which will be the youngest since all samples have a stored span of 10. This will repeat for the 6th-10th items (the calculated span increasing by 10 per item), always replacing the most recently inserted sample. I think the span stored in a sample should be the span calculated when comparing to existing samples, and not the object's byte size. * When a sample is removed from the queue because a sample with a larger span is being added, the span of the removed node is not handed to the younger neighbor, this only happens when a sample is removed due to GC. This means that the span will be given to the next sample added to the queue. When the sample being removed is the youngest sample, this is fine, but when it's a sample that has a younger neighbor, the span should probably be given to that neighbor rather than the newcomer. Handing it to the newcomer gives the new sample a high weight it doesn't deserve. It ends up covering not just the span to the older neighbor, but also the span of the removed node, which doesn't seem to be what we want? I think when a sample is removed because it's being evicted in favor of a new sample, its span should be given to the younger neighbor. If there is no younger neighbor, the span should instead be given to the sample being added.

2 3

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: 8331877: JFR: Remove JIInliner framework
by Erik Gahlin 31 May '24

31 May '24

Could I have a review of a change that removes obsolete classes used for bytecode transformation of the JDK. Testing: test/jdk/jdk/jfr + tier1 Thanks Erik ------------- Commit messages: - Remove instrumentation export - Initial Changes: https://git.openjdk.org/jdk/pull/19483/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19483&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8331877 Stats: 1209 lines in 14 files changed: 225 ins; 981 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19483.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19483/head:pull/19483 PR: https://git.openjdk.org/jdk/pull/19483

2 2

RFR: 8331876: JFR: Move file read and write events to java.base
by Erik Gahlin 30 May '24

30 May '24

Hi, Could I have a review of a change that moves the jdk.FileRead and jdk.FileWrite events to java.base to remove the use of the ASM instrumentation. Testing: jdk/jdk/jfr Thanks Erik ------------- Commit messages: - Update comment - Initial Changes: https://git.openjdk.org/jdk/pull/19129/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19129&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8331876 Stats: 1241 lines in 19 files changed: 567 ins; 651 del; 23 mod Patch: https://git.openjdk.org/jdk/pull/19129.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19129/head:pull/19129 PR: https://git.openjdk.org/jdk/pull/19129

5 29

RFR: 8332880: JFR GCHelper class recognizes "Archive" regions as valid
by Thomas Schatzl 29 May '24

29 May '24

Hi all, please review this small change to remove "Archive" regions from the possible types of the heap region type change events. We removed Archive regions a long time ago. Testing: tier5 (containing the JFR tests afaik) Thanks, Thomas ------------- Commit messages: - 8332880 Changes: https://git.openjdk.org/jdk/pull/19390/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19390&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8332880 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19390.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19390/head:pull/19390 PR: https://git.openjdk.org/jdk/pull/19390

3 4

RecordedObject toString stack depth
by Stig Rohde Døssing 28 May '24

28 May '24

Hi, JFR has an internal pretty printer (PrettyWriter), which is used to implement a friendly toString for RecordedObject. The toString method hardcodes the stack depth to 5 https://github.com/openjdk/jdk/blob/da6aa2a86c86ba5fce747b36dcb2d6001cfcc44… I'm programmatically running JFR and dumping events into a logging system, and figure you intend for people to write their own pretty printers, but I was wondering why this limit is even there? It's already possible to limit the stack depth of recordings via a JFR setting, so why does the toString method need a separate fixed limit?

1 0

RFR: 8326012: JFR: Event for safepoint timeout
by Denghui Dong 27 May '24

27 May '24

There are now some JFR events related to safepoint. When time-to-safepoint (aka ttsp) is too long, these events could not be very helpful since based on them we cannot know which threads cause it and what those threads are doing. Users can use `-XX:+SafepointTimeout -XX:SafepointTimeoutDelay=100` to see the threads that don't reach safepoint in time but without stack traces. Using `-XX:+ AbortVMOnSafepointTimeout` can capture the stack traces but it crashes the process, hence it's not sensible to enable the flag in production. This patch adds a new JFR event `EventSafepoint` to record the threads that causes ttsp too long. This event includes two fields: - safepointId: the relevant safepoint id - timeExceeded: the amount of time exceeding `SafepointTimeoutDelay` used by the thread to reach safepoint In the current version, this event records the stack of those problematic threads when they finally reach safepoint. Hence, there is a bias, but it's still helpful to deduce the root place. A better implementation is to record a more accurate stack, but this will increase complexity. At the same time, the native stack may also be important for this problem, but it is not currently supported by JFR. Any input would be greatly appreciated. ------------- Commit messages: - update - 8326012: JFR: Event for safepoint timeout Changes: https://git.openjdk.org/jdk/pull/17888/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17888&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8326012 Stats: 143 lines in 7 files changed: 136 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/17888.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17888/head:pull/17888 PR: https://git.openjdk.org/jdk/pull/17888

3 24

RFR: 8316328: Shenandoah: jdk/jfr/event/oldobject/TestSanityDefault.java times out for some heap sizes
by Sergey Bylokhov 23 May '24

23 May '24

This is a request to limit the memory used by the test, otherwise it could run "forever" on huge heaps. ------------- Commit messages: - Update TestSanityDefault.java - 8316328: Shenandoah: jdk/jfr/event/oldobject/TestSanityDefault.java times out for some heap sizes Changes: https://git.openjdk.org/jdk/pull/19163/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19163&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8316328 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19163.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19163/head:pull/19163 PR: https://git.openjdk.org/jdk/pull/19163

4 9

RFR: 8328572: JFR: Use Class.forPrimitiveName(String)
by Erik Gahlin 17 May '24

17 May '24

Could I have a review of change that use Class.forPrimitiveName(String) instead checking against every primitive type. Testing: jdk/jdk/jfr Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/19235/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19235&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8328572 Stats: 73 lines in 1 file changed: 13 ins; 56 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/19235.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19235/head:pull/19235 PR: https://git.openjdk.org/jdk/pull/19235

3 5