January 2022 - 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: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable
by Andrey Turbanov 19 May '22

19 May '22

Method `Class.isAssignableFrom` is often used in form of: if (clazz.isAssignableFrom(obj.getClass())) { Such condition could be simplified to more shorter and performarnt code if (clazz.isInstance(obj)) { Replacement is equivalent if it's known that `obj != null`. In JDK codebase there are many such places. I tried to measure performance via JMH Class<?> integerClass = Integer.class; Class<?> numberClass = Number.class; Object integerObject = 45666; Object doubleObject = 8777d; @Benchmark public boolean integerInteger_isInstance() { return integerClass.isInstance(integerObject); } @Benchmark public boolean integerInteger_isAssignableFrom() { return integerClass.isAssignableFrom(integerObject.getClass()); } @Benchmark public boolean integerDouble_isInstance() { return integerClass.isInstance(doubleObject); } @Benchmark public boolean integerDouble_isAssignableFrom() { return integerClass.isAssignableFrom(doubleObject.getClass()); } @Benchmark public boolean numberDouble_isInstance() { return numberClass.isInstance(doubleObject); } @Benchmark public boolean numberDouble_isAssignableFrom() { return numberClass.isAssignableFrom(doubleObject.getClass()); } @Benchmark public boolean numberInteger_isInstance() { return numberClass.isInstance(integerObject); } @Benchmark public boolean numberInteger_isAssignableFrom() { return numberClass.isAssignableFrom(integerObject.getClass()); } @Benchmark public void numberIntegerDouble_isInstance(Blackhole bh) { bh.consume(numberClass.isInstance(integerObject)); bh.consume(numberClass.isInstance(doubleObject)); } @Benchmark public void integerIntegerDouble_isAssignableFrom(Blackhole bh) { bh.consume(integerClass.isAssignableFrom(integerObject.getClass())); bh.consume(integerClass.isAssignableFrom(doubleObject.getClass())); } `isInstance` is a bit faster than `isAssignableFrom` Benchmark Mode Cnt Score Error Units integerDouble_isAssignableFrom avgt 5 1,173 ± 0,026 ns/op integerDouble_isInstance avgt 5 0,939 ± 0,038 ns/op integerIntegerDouble_isAssignableFrom avgt 5 2,106 ± 0,068 ns/op numberIntegerDouble_isInstance avgt 5 1,516 ± 0,046 ns/op integerInteger_isAssignableFrom avgt 5 1,175 ± 0,029 ns/op integerInteger_isInstance avgt 5 0,886 ± 0,017 ns/op numberDouble_isAssignableFrom avgt 5 1,172 ± 0,007 ns/op numberDouble_isInstance avgt 5 0,891 ± 0,030 ns/op numberInteger_isAssignableFrom avgt 5 1,169 ± 0,014 ns/op numberInteger_isInstance avgt 5 0,887 ± 0,016 ns/op ------------- Commit messages: - [PATCH] Use Class.isInstance instead of Class.isAssignableFrom where applicable Changes: https://git.openjdk.java.net/jdk/pull/7061/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7061&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280035 Stats: 25 lines in 10 files changed: 0 ins; 3 del; 22 mod Patch: https://git.openjdk.java.net/jdk/pull/7061.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7061/head:pull/7061 PR: https://git.openjdk.java.net/jdk/pull/7061

6 9

RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder)
by Tyler 17 Feb '22

17 Feb '22

Just in time for the holidays I have completed an implementation of the JFR functionality for AIX. As a side note, this is my first submission to OpenJDK 👋 ### Implementation notes and alternatives considered After modifying the build system to allow the --enable-jvm-feature-jfr to work on AIX, my task was to implement the interfaces from os_perf.hpp. The os_perf_aix.cpp implementation that existed was, I believe, a copy of the Linux implementation. A review of the code in that file showed that NetworkInterface, CPUPerformanceInterface, and SystemProcessInterface would require modification to work on AIX. Using the Linux implementation as a guide, I initially expected to use files from the aix procfs like /proc/<pid>/psinfo, and /proc/<pid>/status in a manner similar to the Linux implementation. However, I ended up using libperfstat for all functionality required by the interfaces. ### Testing Testing for JFR seems to be quite light in the repo both before and after this change. In addition to performing manual testing, I have added some basic sanity checks that ensure events can be created and committed (using jtreg), and performs some basic checks on the results of the interface member functions (using gtest). ### More notes I've sent an email to the JFR group about a TOC overflow warning I encountered while building (for the release server target). I believe the fix is to pass -qpic=large when using the xlc toolchain, but my modifications to flags-cflags.m4 have not been successful in removing this warning. ------------- Commit messages: - 8203290: Implements Java Flight Recorder on AIX Changes: https://git.openjdk.java.net/jdk/pull/6885/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6885&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8203290 Stats: 1090 lines in 7 files changed: 425 ins; 500 del; 165 mod Patch: https://git.openjdk.java.net/jdk/pull/6885.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6885/head:pull/6885 PR: https://git.openjdk.java.net/jdk/pull/6885

11 92

RFR: 8279613: JFR: Snippify Javadoc
by Erik Gahlin 07 Feb '22

07 Feb '22

Hi, Could I have a review that uses the new snippet tag described in https://openjdk.java.net/jeps/413 I found a couple of issues with the current documentation, but to simplify review I kept the code samples as is. I will improve documentation separately. I added package "example1" and "example2" to the Snippet file to avoid collisions and complaints about code not being in a package (at least when using Eclipse). Testing: tier1 + jdk/jdk/jfr Thanks Erik ------------- Commit messages: - Unique package name - Remove trailing whitespace - initial Changes: https://git.openjdk.java.net/jdk/pull/7157/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7157&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279613 Stats: 711 lines in 15 files changed: 412 ins; 272 del; 27 mod Patch: https://git.openjdk.java.net/jdk/pull/7157.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7157/head:pull/7157 PR: https://git.openjdk.java.net/jdk/pull/7157

2 4

Re: RFR: 8280684: JfrRecorderService failes with guarantee(num_written > 0) when no space left on device.
by David Holmes 28 Jan '22

28 Jan '22

On Wed, 26 Jan 2022 06:41:41 GMT, KIRIYAMA Takuya <duke(a)openjdk.java.net> wrote: > I think JFR should report an error message and jvm should shut down safely instead of gurantee failure. > > For instance, jdk.jfr.internal.Repository#newChunk() reports an appropriate message and stops jvm as below > by using JfrJavaSupport::abort(). > > [0.673s][error][jfr] Could not create chunk in repository /tmp/2022_01_12_22_32_42_18030, class java.io.IOException: Unable to create JFR repository directory using base location (/tmp) > [0.673s][error][jfr,system] Could not create chunk in repository /tmp/2022_01_12_22_32_42_18030, class java.io.IOException: Unable to create JFR repository directory using base location (/tmp) > [0.673s][error][jfr,system] An irrecoverable error in Jfr. Shutting down VM... > > I modified StreamWriterHost not to call guarantee failure but to call JfrJavaSupport::abort(). > I added a argument to JfrJavaSupport::abort() which tells os::abort() not to put out core > because there is no space on device. > Could you please review the fix? JFR team need to review this. ------------- PR: https://git.openjdk.java.net/jdk/pull/7227

1 0

RFR: 8266410: jdk/jfr/javaagent/TestLoadedAgent.java failed with "Mismatch in TestEvent count"
by Erik Gahlin 19 Jan '22

19 Jan '22

Hi, Could I have a review of a test fix. The test emits a lot of data in a short period of time. On my machine about 35 MB/s, which should be fine in most cases, but could potentially lead to events being dropped. I could reproduce the issue, with 4% of all events being dropped and with jdk.DataLoss events in the recording, if increased the number of events significantly. Contributing to the flakiness is the default recording and its periodic events that uses buffers at the same time. I think it is fine to run with the default recording, as it is more realistic, but we should not do so with too many events emitted per second. Purpose of the test is not to stress the buffer system, but to see we can load JFR with a Java agent (with some user-defined events). I changed the test so it doesn't read all event into memory to make it easier to reproduce the issue without using too much RAM. I choose not to use EventStream.openFile(Path), because we might want to run this with JDK 11 where event streaming is not available. I printed the recording size and any dataloss events to make it easier to diagnose if it should reoccur. Testing: jdk/jdk/jfr/javaagent/* 100 times Thanks Erik ------------- Commit messages: - Initial Changes: https://git.openjdk.java.net/jdk/pull/7141/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7141&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266410 Stats: 18 lines in 1 file changed: 13 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/7141.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7141/head:pull/7141 PR: https://git.openjdk.java.net/jdk/pull/7141

2 2

RFR: 8280189: JFR: TestPrintXML should print mismatching XML
by Erik Gahlin 19 Jan '22

19 Jan '22

Hi, Could I have a review of this improvement to TestPrintXML? Testing: jdk/jfr/tool/TestPrintXML.java Thanks Erik ------------- Commit messages: - Remove unused imports - More robust - Fixes - Formatting and copyright year - Initial Changes: https://git.openjdk.java.net/jdk/pull/7140/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7140&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280189 Stats: 23 lines in 1 file changed: 19 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/7140.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7140/head:pull/7140 PR: https://git.openjdk.java.net/jdk/pull/7140

2 2

RFR: JDK-8280157: wrong texts Falied in a couple of tests
by Matthias Baesken 19 Jan '22

19 Jan '22

Very small change fixing wrong strings "Falied" in a couple of tests. ------------- Commit messages: - JDK-8280157 Changes: https://git.openjdk.java.net/jdk/pull/7142/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7142&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280157 Stats: 9 lines in 5 files changed: 0 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/7142.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7142/head:pull/7142 PR: https://git.openjdk.java.net/jdk/pull/7142

3 3

RFR: 8280055: JFR: Improve ObjectContext implementation
by Erik Gahlin 18 Jan '22

18 Jan '22

Hi, Could I have review of an enhancement that improves the ObjectContext class. For details, se bug. I also turned the UnsignedValue class into a record class. Testing: jdk/jdk/jfr Thanks Erik ------------- Commit messages: - Update comment and copyright - Initial Changes: https://git.openjdk.java.net/jdk/pull/7102/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7102&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280055 Stats: 43 lines in 2 files changed: 18 ins; 11 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/7102.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7102/head:pull/7102 PR: https://git.openjdk.java.net/jdk/pull/7102

2 2

RFR: 8280058: JFR: StreamUtils::getJfrRepository(Process) should print stdout and stderr
by Erik Gahlin 18 Jan '22

18 Jan '22

Hi, Could I have review of a test change that makes it easier diagnose why tests such as TestOutOfProcessMigration.java and TestJVMCrash.java fails. Testing: jdk/jdk/jfr Thanks Erik ------------- Commit messages: - Typo - Initial Changes: https://git.openjdk.java.net/jdk/pull/7101/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7101&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280058 Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7101.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7101/head:pull/7101 PR: https://git.openjdk.java.net/jdk/pull/7101

2 2