August 2025 - hotspot-jfr-dev

RFR: 8365611: Throttle any JFR event
by Thomas Stuefe 26 Aug '25

26 Aug '25

This patch prepares the way to throttle any JFR event, not only the three events for which we hard-code throttlers. It also cleans up the throttler code and replaces the switch construct with a lookup table. The patch does not yet enable throttling for any other event. I can do that, if we agree that this is okay. But I'd like to avoid arguments, so for now the patch does not add the ability to throttle to any event; it just makes it trivially easy to do that if needed. ------------- Commit messages: - Update jfrEventThrottler.cpp - wip - fixes - start Changes: https://git.openjdk.org/jdk/pull/26801/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=26801&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8365611 Stats: 84 lines in 2 files changed: 42 ins; 22 del; 20 mod Patch: https://git.openjdk.org/jdk/pull/26801.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/26801/head:pull/26801 PR: https://git.openjdk.org/jdk/pull/26801

4 15

Re: RFR: 8365306: Provide OS Process Size and Libc statistic metrics to JFR
by Erik Gahlin 26 Aug '25

26 Aug '25

On Wed, 13 Aug 2025 09:42:57 GMT, Thomas Stuefe <stuefe(a)openjdk.org> wrote: > This provides the following new metrics: > - `ProcessSize` event (new, periodic) > - vsize (for analyzing address-space fragmentation issues) > - RSS including subtypes (subtypes are useful for excluding atypical issues, e.g. kernel problems that cause large file buffer bloat) > - peak RSS > - process swap (if we swap we cannot trust the RSS values, plus it indicates bad sizing) > - pte size (to quickly see if we run with a super-large working set but an unsuitably small page size) > - `LibcStatistics` (new, periodic) > - outstanding malloc size (important counterpoint to whatever NMT tries to tell me, which alone is often misleading) > - retained malloc size (super-important for the same reason) > - number of libc trims the hotspot executed (needed to gauge the usefulness of the retain counter, and to see if a customer employs native heap auto trimming (`-XX:TrimNativeHeapInterval`) > - `NativeHeapTrim` (new, event-driven) (for both manual and automatic trims) > - RSS before and RSS after > - RSS recovered by this trim > - whether it was an automatic or manual trim > - duration > - `JavaThreadStatistic` > - os thread counter (new field) (useful to understand the behavior of third-party code in our process if threads are created that bypass the JVM. E.g. some custom launchers do that.) > - nonJava thread counter (new field) (needed to interprete the os thread counter) > > Notes: > - we already have `ResidentSetSize` event, and the new `ProcessSize` event is a superset of that. I don't know how these cases are handled. I'd prefer to throw the old event out, but JMC has a hard-coded chart for RSS, so I kept it in unless someone tells me to remove it. > > - Obviously, the libc events are very platform-specific. Still, I argue that these metrics are highly useful. We want people to use JFR and JMC; people include developers that are dealing with performance problems that require platform-specific knowledge to understand. See my comment in the JBS issue. > > I provided implementations, as far as possible, to Linux, MacOS and Windows. > > Testing: > - ran the new tests manually and as part of GHAs Events can be viewed as tables in a relational database. Duplicating information or designing tables based on the user interface is not a good idea. Correlating or grouping data from different events is not a new problem. The way we have solved it in the past is by using an explicit ID, similar to a foreign key (gcId, compilerId, safepointId etc), or by using the same timestamp (ActiveSetting, NativeMemory* etc.). In this case, I think a timestamp makes more sense. Tools can place the value on a timeline and have memory on the y-axis. An alternative design is to do something similar to the NativeMemoryUsage event. That is, to split the data per type <Event name="ProcessMemoryUsage"> <Field type="MemoryType" name="type" label="Type" /> <Field type="ulong" contentType="bytes" name="amount" label="Amount"/> </Event> and then emit what is supported on a specific platform and have all those event with the same timestamp. I'm not sure which of the following memory types make sense to include, there is also a maintenance aspect, or if it the list is correct or complete, but the data would be normalized, even if it differs depending on the platform. Memory Type Linux macOS Windows Description -------------------------- ------ -------- --------- ----------------------------------------------------------' Resident Set Size Yes Yes - Resident memory currently in physical RAM Private Bytes - - Yes Privately allocated memory Shared Memory Yes Yes Yes Memory shared among processes Virtual Memory Size Yes Yes Yes Total reserved address space Commit Charge - - Yes Promised/guaranteed memory for process Mapped File Memory Yes Yes Yes Memory mapped from files Page Table Size Yes - - Memory used for page tables Swap / Swapped Yes Yes Yes Memory swapped out to disk Wired Memory - Yes - Locked memory that can't be paged out Compressed Memory - Yes - Memory compressed by the system Paged Pool - - Yes Pool for pageable kernel allocations Non-paged Pool - - Yes Pool for non-pageable kernel allocations Working Set - - Yes Memory pages currently in RAM for the process Stack Yes Yes Yes Memory used by call stacks Heap / Malloc Yes Yes Yes Dynamically allocated memory (heap) Image Yes Yes - Memory used for executables and loaded libraries ------------- PR Comment: https://git.openjdk.org/jdk/pull/26756#issuecomment-3224085095

1 0

Re: RFR: 8365306: Provide OS Process Size and Libc statistic metrics to JFR
by Erik Gahlin 26 Aug '25

26 Aug '25

On Wed, 13 Aug 2025 09:42:57 GMT, Thomas Stuefe <stuefe(a)openjdk.org> wrote: > This provides the following new metrics: > - `ProcessSize` event (new, periodic) > - vsize (for analyzing address-space fragmentation issues) > - RSS including subtypes (subtypes are useful for excluding atypical issues, e.g. kernel problems that cause large file buffer bloat) > - peak RSS > - process swap (if we swap we cannot trust the RSS values, plus it indicates bad sizing) > - pte size (to quickly see if we run with a super-large working set but an unsuitably small page size) > - `LibcStatistics` (new, periodic) > - outstanding malloc size (important counterpoint to whatever NMT tries to tell me, which alone is often misleading) > - retained malloc size (super-important for the same reason) > - number of libc trims the hotspot executed (needed to gauge the usefulness of the retain counter, and to see if a customer employs native heap auto trimming (`-XX:TrimNativeHeapInterval`) > - `NativeHeapTrim` (new, event-driven) (for both manual and automatic trims) > - RSS before and RSS after > - RSS recovered by this trim > - whether it was an automatic or manual trim > - duration > - `JavaThreadStatistic` > - os thread counter (new field) (useful to understand the behavior of third-party code in our process if threads are created that bypass the JVM. E.g. some custom launchers do that.) > - nonJava thread counter (new field) (needed to interprete the os thread counter) > > Notes: > - we already have `ResidentSetSize` event, and the new `ProcessSize` event is a superset of that. I don't know how these cases are handled. I'd prefer to throw the old event out, but JMC has a hard-coded chart for RSS, so I kept it in unless someone tells me to remove it. > > - Obviously, the libc events are very platform-specific. Still, I argue that these metrics are highly useful. We want people to use JFR and JMC; people include developers that are dealing with performance problems that require platform-specific knowledge to understand. See my comment in the JBS issue. > > I provided implementations, as far as possible, to Linux, MacOS and Windows. > > Testing: > - ran the new tests manually and as part of GHAs What is the problem with adding RSS metrics to the existing ResidentSetSize event? ------------- PR Comment: https://git.openjdk.org/jdk/pull/26756#issuecomment-3199438538

3 5

JFR: Scrubbing sensitive information from events
by Erwan Viollet 25 Aug '25

25 Aug '25

Hello, I am currently looking into how to remove sensitive information from JFR events. The main events that typically contain sensitive information: jdk.SystemProcess, jdk.InitialSystemProperty, jdk.JVMInformation. Passwords from command lines can typically be found in these events. Dropping these events altogether is not ideal, as we need them to make relevant performance recommendations to users (e.g. suggesting JVM or system setting adjustments). Dropping them or scrubbing them on the backend side (after the fact) requires decompressing and re-writing these events, which is wasteful in terms of both compute and storage. The approach is not perfect, as we still end up intaking and temporarily storing sensitive information. Ideally, we would like to be able to scrub or redact only the sensitive fields within these events (for example, using a simple regex or pattern-based rule), rather than dropping the whole event. We also want to avoid handling this only after the event has already been written to the JFR file, as that does not fully mitigate the risk of exposing sensitive data. At present, it appears there is no public API or supported mechanism to intercept or scrub JFR events in-process, before they are persisted. What would you think of an API accepting custom scrubbing patterns so that sensitive data never leaves the JVM in an unredacted state? Are there any plans or discussions in this area? I am fairly new to the JFR world, so it is likely that I missed previous discussions around this. Thank you, Best regards, Erwan Viollet, Profiling team, Datadog

2 5

RFR: 8365815: JFR: Update metadata.xml with 'jfr query' examples
by Erik Gahlin 25 Aug '25

25 Aug '25

Could I have a review of a PR that adds a couple of 'jfr query' examples to metadata.xml? It turns out that the EventDuration event doesn't work because it has no fields. The fix is to remove the class-specific commit() method and constructor for events without fields. Testing: test/jdk/jdk/jfr + tier1 Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/26869/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=26869&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8365815 Stats: 15 lines in 2 files changed: 13 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/26869.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/26869/head:pull/26869 PR: https://git.openjdk.org/jdk/pull/26869

2 4

Withdrawn: 8365604: Null pointer dereference in src/hotspot/share/adlc/output_h.cpp ArchDesc::declareClasses()
by Artem Semenov 22 Aug '25

22 Aug '25

On Fri, 15 Aug 2025 11:58:48 GMT, Artem Semenov <asemenov(a)openjdk.org> wrote: > The defect has been detected and confirmed in the function ArchDesc::declareClasses() located in the file src/hotspot/share/adlc/output_h.cpp with static code analysis. This defect can potentially lead to a null pointer dereference. > > The pointer instr->_matrule is dereferenced in line 1952 without checking for nullptr, although earlier in line 1858 the same pointer is checked for nullptr, which indicates that it can be null. > > According to [this](https://github.com/openjdk/jdk/pull/26002#issuecomment-3023050372) comment, this PR contains fixes for similar cases in other places. This pull request has been closed without being integrated. ------------- PR: https://git.openjdk.org/jdk/pull/26798

1 0

Re: RFR: 8365604: Null pointer dereference in src/hotspot/share/adlc/output_h.cpp ArchDesc::declareClasses()
by David Holmes 22 Aug '25

22 Aug '25

On Fri, 15 Aug 2025 11:58:48 GMT, Artem Semenov <asemenov(a)openjdk.org> wrote: > The defect has been detected and confirmed in the function ArchDesc::declareClasses() located in the file src/hotspot/share/adlc/output_h.cpp with static code analysis. This defect can potentially lead to a null pointer dereference. > > The pointer instr->_matrule is dereferenced in line 1952 without checking for nullptr, although earlier in line 1858 the same pointer is checked for nullptr, which indicates that it can be null. > > According to [this](https://github.com/openjdk/jdk/pull/26002#issuecomment-3023050372) comment, this PR contains fixes for similar cases in other places. Some alignment nits where you have added additional condition clauses. Some of these are difficult to evaluate in isolation and will need review from the specific component areas. src/hotspot/share/adlc/output_h.cpp line 1952: > 1950: }*/ > 1951: else if( instr->is_ideal_copy() && > 1952: (instr->_matrule != nullptr && instr->_matrule->_rChild != nullptr) && Suggestion: (instr->_matrule != nullptr && instr->_matrule->_rChild != nullptr) && src/hotspot/share/c1/c1_LinearScan.cpp line 4422: > 4420: > 4421: if ((cur != nullptr) && > 4422: (cur->from() < split_pos)) { Suggestion: (cur->from() < split_pos)) { src/hotspot/share/nmt/mallocSiteTable.cpp line 172: > 170: index < pos_idx && head != nullptr; > 171: index++, head = ((MallocSiteHashtableEntry*)head->next() == nullptr) ? head : > 172: (MallocSiteHashtableEntry*)head->next()) {} This doesn't look right to me. We check `head != nullptr` in the loop condition so we cannot reach the assignment if it is null. src/hotspot/share/opto/vectorIntrinsics.cpp line 1319: > 1317: log_if_needed(" ** not supported: arity=%d op=%s vlen=%d etype=%s atype=%s ismask=no", > 1318: is_scatter, is_scatter ? "scatter" : "gather", > 1319: num_elem, type2name(elem_bt), type2name(arr_type->elem()->array_element_basic_type())); There is a bug here but I'm not sure it is what you think it is. ------------- Changes requested by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/26798#pullrequestreview-3136325292 PR Review Comment: https://git.openjdk.org/jdk/pull/26798#discussion_r2287976814 PR Review Comment: https://git.openjdk.org/jdk/pull/26798#discussion_r2287984002 PR Review Comment: https://git.openjdk.org/jdk/pull/26798#discussion_r2287993050 PR Review Comment: https://git.openjdk.org/jdk/pull/26798#discussion_r2287996530

3 8

Re: RFR: 8365604: Null pointer dereference in src/hotspot/share/adlc/output_h.cpp ArchDesc::declareClasses() [v2]
by Artem Semenov 22 Aug '25

22 Aug '25

> The defect has been detected and confirmed in the function ArchDesc::declareClasses() located in the file src/hotspot/share/adlc/output_h.cpp with static code analysis. This defect can potentially lead to a null pointer dereference. > > The pointer instr->_matrule is dereferenced in line 1952 without checking for nullptr, although earlier in line 1858 the same pointer is checked for nullptr, which indicates that it can be null. > > According to [this](https://github.com/openjdk/jdk/pull/26002#issuecomment-3023050372) comment, this PR contains fixes for similar cases in other places. Artem Semenov has updated the pull request incrementally with two additional commits since the last revision: - Update src/hotspot/share/c1/c1_LinearScan.cpp Co-authored-by: David Holmes <62092539+dholmes-ora(a)users.noreply.github.com> - Update src/hotspot/share/adlc/output_h.cpp Co-authored-by: David Holmes <62092539+dholmes-ora(a)users.noreply.github.com> ------------- Changes: - all: https://git.openjdk.org/jdk/pull/26798/files - new: https://git.openjdk.org/jdk/pull/26798/files/80777ced..dd21148b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=26798&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=26798&range=00-01 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/26798.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/26798/head:pull/26798 PR: https://git.openjdk.org/jdk/pull/26798

3 2

Re: RFR: 8365604: Null pointer dereference in src/hotspot/share/adlc/output_h.cpp ArchDesc::declareClasses()
by David Holmes 21 Aug '25

21 Aug '25

On Fri, 15 Aug 2025 11:58:48 GMT, Artem Semenov <asemenov(a)openjdk.org> wrote: > The defect has been detected and confirmed in the function ArchDesc::declareClasses() located in the file src/hotspot/share/adlc/output_h.cpp with static code analysis. This defect can potentially lead to a null pointer dereference. > > The pointer instr->_matrule is dereferenced in line 1952 without checking for nullptr, although earlier in line 1858 the same pointer is checked for nullptr, which indicates that it can be null. > > According to [this](https://github.com/openjdk/jdk/pull/26002#issuecomment-3023050372) comment, this PR contains fixes for similar cases in other places. I've added some additional mailing lists to ensure better coverage here. Also I think you need to update the JBS (and PR) title to reflect the broader scope of the changes. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26798#issuecomment-3206112684

2 1

RFR: 8365610: Sort share/jfr includes
by Francesco Andreuzzi 20 Aug '25

20 Aug '25

This PR sorts the includes in hotspot/share/jfr using SortIncludes.java. I'm also adding the directory to TestIncludesAreSorted. Passes tier1. ------------- Commit messages: - reorder Changes: https://git.openjdk.org/jdk/pull/26800/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=26800&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8365610 Stats: 173 lines in 56 files changed: 82 ins; 89 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/26800.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/26800/head:pull/26800 PR: https://git.openjdk.org/jdk/pull/26800

4 22