August 2023 - hotspot-jfr-dev

RFR: 8288936: Wrong lock ordering writing G1HeapRegionTypeChange JFR event
by Markus Grönlund 09 Aug '23

09 Aug '23

Greetings, Please help review this adjustment to two JFR locks to avoid lock-ordering problems. Test: jdk_jfr, reproducers Thanks Markus ------------- Commit messages: - 8288936 Changes: https://git.openjdk.org/jdk/pull/15155/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15155&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8288936 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/15155.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15155/head:pull/15155 PR: https://git.openjdk.org/jdk/pull/15155

2 2

RFR: 8313891: JFR: Incorrect exception message for RecordedObject::getInt
by Erik Gahlin 09 Aug '23

09 Aug '23

Could I have a review of PR that fixes an incorrect exception message. Testing: test/jdk/jdk/jfr Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.org/jdk/pull/15181/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15181&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8313891 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15181.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15181/head:pull/15181 PR: https://git.openjdk.org/jdk/pull/15181

2 2

Re: RFR: 8313722: JFR: Avoid unnecessary calls to Events.from(Recording) [v2]
by Erik Gahlin 08 Aug '23

08 Aug '23

> Could I have a review of PR that removes unnecessary calls to Events.from(Recording). > > With each call a new dump file is created. A better approach is store the events in a List that can be reused, > > Testing: jdk/jdk/jfr > > Thanks > Erik Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision: Fix typos ------------- Changes: - all: https://git.openjdk.org/jdk/pull/15147/files - new: https://git.openjdk.org/jdk/pull/15147/files/41fedb99..6ee72814 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15147&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15147&range=00-01 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/15147.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15147/head:pull/15147 PR: https://git.openjdk.org/jdk/pull/15147

2 1

Integrated: 8313552: Fix -Wconversion warnings in JFR code
by Markus Grönlund 08 Aug '23

08 Aug '23

On Fri, 4 Aug 2023 10:55:04 GMT, Markus Grönlund <mgronlun(a)openjdk.org> wrote: > Greetings, > > this change set removes -Wconversion warnings in the JFR code. > > Testing: jdk_jfr > > Thanks > Markus This pull request has now been integrated. Changeset: 091e65e9 Author: Markus Grönlund <mgronlun(a)openjdk.org> URL: https://git.openjdk.org/jdk/commit/091e65e95b42f7c425b1a39ee518230d4e8bb05c Stats: 250 lines in 28 files changed: 21 ins; 11 del; 218 mod 8313552: Fix -Wconversion warnings in JFR code Reviewed-by: coleenp ------------- PR: https://git.openjdk.org/jdk/pull/15152

1 0

Re: RFR: 8313552: Fix -Wconversion warnings in JFR code [v4]
by Markus Grönlund 05 Aug '23

05 Aug '23

> Greetings, > > this change set removes -Wconversion warnings in the JFR code. > > Testing: jdk_jfr > > Thanks > Markus Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision: revert to static_cast ------------- Changes: - all: https://git.openjdk.org/jdk/pull/15152/files - new: https://git.openjdk.org/jdk/pull/15152/files/9fa7a46e..11d4953f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15152&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15152&range=02-03 Stats: 49 lines in 7 files changed: 0 ins; 0 del; 49 mod Patch: https://git.openjdk.org/jdk/pull/15152.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15152/head:pull/15152 PR: https://git.openjdk.org/jdk/pull/15152

1 0

Integrated: 8313141: Missing check for os_thread type in os_windows.cpp
by Julian Waters 05 Aug '23

05 Aug '23

On Sat, 29 Jul 2023 05:13:24 GMT, Julian Waters <jwaters(a)openjdk.org> wrote: > 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 This pull request has now been integrated. Changeset: 90d795ab Author: Julian Waters <jwaters(a)openjdk.org> URL: https://git.openjdk.org/jdk/commit/90d795abf10bf8b8b53079c1afd19fee7b4cb6cf Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8313141: Missing check for os_thread type in os_windows.cpp Reviewed-by: dholmes, mgronlun ------------- PR: https://git.openjdk.org/jdk/pull/15079

1 0

Re: RFR: 8313552: Fix -Wconversion warnings in JFR code [v3]
by Markus Grönlund 04 Aug '23

04 Aug '23

> Greetings, > > this change set removes -Wconversion warnings in the JFR code. > > Testing: jdk_jfr > > Thanks > Markus Markus Grönlund has updated the pull request incrementally with one additional commit since the last revision: writer casts nonreversible ------------- Changes: - all: https://git.openjdk.org/jdk/pull/15152/files - new: https://git.openjdk.org/jdk/pull/15152/files/5c9e1ffe..9fa7a46e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15152&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15152&range=01-02 Stats: 39 lines in 1 file changed: 0 ins; 0 del; 39 mod Patch: https://git.openjdk.org/jdk/pull/15152.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15152/head:pull/15152 PR: https://git.openjdk.org/jdk/pull/15152

2 1

Re: RFR: 8313552: Fix -Wconversion warnings in JFR code
by Coleen Phillimore 04 Aug '23

04 Aug '23

On Fri, 4 Aug 2023 10:55:04 GMT, Markus Grönlund <mgronlun(a)openjdk.org> wrote: > Greetings, > > this change set removes -Wconversion warnings in the JFR code. > > Testing: jdk_jfr > > Thanks > Markus Thank you so much for fixing these warnings. I had one or two suggestions for changes. src/hotspot/share/jfr/instrumentation/jfrEventClassTransformer.cpp line 914: > 912: u4 stack_map_attrib_len = writer.current_offset() - stack_map_attrib_len_offset; > 913: // the stack_map_table_attributes_length value is exclusive > 914: stack_map_attrib_len -= sizeof(u4); I'm thinking we should have globalDefinitions.hpp: const int u4_size = (int)sizeof(u4); const int u2_size = (int)sizeof(u2); const int u1_size = (int)sizeof(u1); I don't think you have to static_cast<u4>(4) here. It looks bad here. src/hotspot/share/jfr/periodic/jfrPeriodic.cpp line 437: > 435: EventGCSurvivorConfiguration event; > 436: event.set_maxTenuringThreshold(static_cast<u1>(conf.max_tenuring_threshold())); > 437: event.set_initialTenuringThreshold(static_cast<u1>(conf.initial_tenuring_threshold())); The Wconversion team has been preferring checked_cast<> for all narrowing casts. src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.hpp line 58: > 56: > 57: template <typename T> > 58: static void meta_clear(uint8_t bits, const T* ptr); jbyte -> uint8_t is definitely preferable. ------------- PR Review: https://git.openjdk.org/jdk/pull/15152#pullrequestreview-1563070398 PR Review Comment: https://git.openjdk.org/jdk/pull/15152#discussion_r1284541315 PR Review Comment: https://git.openjdk.org/jdk/pull/15152#discussion_r1284542564 PR Review Comment: https://git.openjdk.org/jdk/pull/15152#discussion_r1284543641

2 4

Re: RFR: 8313552: Fix -Wconversion warnings in JFR code [v2]
by Markus Grönlund 04 Aug '23

04 Aug '23

> Greetings, > > this change set removes -Wconversion warnings in the JFR code. > > Testing: jdk_jfr > > Thanks > Markus Markus Grönlund has updated the pull request incrementally with two additional commits since the last revision: - remove compile flag - checked_cast ------------- Changes: - all: https://git.openjdk.org/jdk/pull/15152/files - new: https://git.openjdk.org/jdk/pull/15152/files/c790ecfa..5c9e1ffe Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15152&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15152&range=00-01 Stats: 95 lines in 10 files changed: 0 ins; 0 del; 95 mod Patch: https://git.openjdk.org/jdk/pull/15152.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15152/head:pull/15152 PR: https://git.openjdk.org/jdk/pull/15152

1 0

Re: RFR: 8313722: JFR: Avoid unnecessary calls to Events.from(Recording)
by Andrey Turbanov 04 Aug '23

04 Aug '23

On Fri, 4 Aug 2023 02:33:48 GMT, Erik Gahlin <egahlin(a)openjdk.org> wrote: > Could I have a review of PR that removes unnecessary calls to Events.from(Recording). > > With each call a new dump file is created. A better approach is store the events in a List that can be reused, > > Testing: jdk/jdk/jfr > > Thanks > Erik test/jdk/jdk/jfr/event/runtime/TestSafepointEvents.java line 71: > 69: try { > 70: // Verify that each event type was seen at least once > 71: List<RecordedEvent> events = Events.fromRecording(recording); Suggestion: List<RecordedEvent> events = Events.fromRecording(recording); ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15147#discussion_r1284463253

1 0