Re: RFR: 8278123: serviceability/dcmd/vm/ClassLoaderStatsTest.java failing with java.lang.AssertionError: Should have a hidden class [v2]
On Thu, 28 Apr 2022 21:58:46 GMT, Leonid Mesnik wrote:
>> The test failed if GC happens somewhere between
>> Class c = Class.forName("TestClass", true, dummyloader);
>> and
>> OutputAnalyzer output = executor.execute("VM.classloader_stats");
>>
>> The fix is to make hc static as Chris proposed.
>>
>> To verfiy fix I add System.gc() before
>> executor.execute("VM.classloader_stats");
>
> Leonid Mesnik has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains three additional
> commits since the last revision:
>
> - comment fixed
> - Merge branch 'master' of https://github.com/openjdk/jdk into 8278123
> - fix
I assume you'll remove this test from the-Xcomp exclude list
(test/hotspot/jtreg/ProblemList-Xcomp.txt) as part of this change,
-
PR: https://git.openjdk.java.net/jdk/pull/8438
RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach()
The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never supported the value `"rw"` since the source code was imported to the openjdk repo more than 15 years ago. In fact HotSpot throws `IllegalArgumentException` when such a mode is specified. It's unlikely such a mode will be required for future enhancements. Support for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` is the only supported mode. I also cleaned up related code in the JDK and HotSpot. Testing: - Passed tiers 1 and 2 - Tiers 3, 4, 5 are in progress - Commit messages: - 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach() Changes: https://git.openjdk.java.net/jdk/pull/8622/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8622&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286441 Stats: 193 lines in 15 files changed: 0 ins; 144 del; 49 mod Patch: https://git.openjdk.java.net/jdk/pull/8622.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8622/head:pull/8622 PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8285739: disable EA when both JVMTI and Preview are enabled
On Mon, 9 May 2022 01:36:39 GMT, Leonid Mesnik wrote: > The fix disables EscapeBarrier and EscapeAnalysis when certain JVMTI > capabilities are enabled and --enable-preview. > > It restores the same behavior as it was before > https://bugs.openjdk.java.net/browse/JDK-8227745 "Enable Escape Analysis for > Better Performance in the Presence of JVMTI Agents" is implemented when > Continuations are enabled. Hi Leonid, your changes look good to me. Thanks, Richard. Ok, thanks for letting me know. - Marked as reviewed by rrich (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8589
Integrated: 8283651: nsk/jvmti/SuspendThread/suspendthrd003 may leak native memory
On Mon, 9 May 2022 20:15:21 GMT, Daniel D. Daugherty wrote: > A trivial fix to nsk_jvmti_threadByName() to solve a native memory leak. > > This fix has been stress tested with my > StressWrapper60M_NMT_detail_suspendthrd003.java > test and I did NMT detailed monitoring for the whole 60M run. Total committed > memory has > gone down from 1.17GB to 300MB. Prior to my last rebase, it was down to 239MB. > > I ran the fix thru Mach5 Tier[1-7] with no failures. I just rebased the fix > to the latest jdk/jdk > and Mach5 Tier[1-3] have passed with no failures. Tier[45] are still running. This pull request has now been integrated. Changeset: 61450bb0 Author:Daniel D. Daugherty URL: https://git.openjdk.java.net/jdk/commit/61450bb061ecda9700ddbd387a1f0659ebd1cced Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod 8283651: nsk/jvmti/SuspendThread/suspendthrd003 may leak native memory Reviewed-by: lmesnik - PR: https://git.openjdk.java.net/jdk/pull/8609
Re: RFR: 8283651: nsk/jvmti/SuspendThread/suspendthrd003 may leak native memory
On Mon, 9 May 2022 20:15:21 GMT, Daniel D. Daugherty wrote: > A trivial fix to nsk_jvmti_threadByName() to solve a native memory leak. > > This fix has been stress tested with my > StressWrapper60M_NMT_detail_suspendthrd003.java > test and I did NMT detailed monitoring for the whole 60M run. Total committed > memory has > gone down from 1.17GB to 300MB. Prior to my last rebase, it was down to 239MB. > > I ran the fix thru Mach5 Tier[1-7] with no failures. I just rebased the fix > to the latest jdk/jdk > and Mach5 Tier[1-3] have passed with no failures. Tier[45] are still running. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/8609
Re: RFR: 8283651: nsk/jvmti/SuspendThread/suspendthrd003 may leak native memory
On Mon, 9 May 2022 21:57:41 GMT, Leonid Mesnik wrote: >> A trivial fix to nsk_jvmti_threadByName() to solve a native memory leak. >> >> This fix has been stress tested with my >> StressWrapper60M_NMT_detail_suspendthrd003.java >> test and I did NMT detailed monitoring for the whole 60M run. Total >> committed memory has >> gone down from 1.17GB to 300MB. Prior to my last rebase, it was down to >> 239MB. >> >> I ran the fix thru Mach5 Tier[1-7] with no failures. I just rebased the fix >> to the latest jdk/jdk >> and Mach5 Tier[1-3] have passed with no failures. Tier[45] are still running. > > Marked as reviewed by lmesnik (Reviewer). @lmesnik - Thanks for the review! Do you agree that this is a trivial fix? - PR: https://git.openjdk.java.net/jdk/pull/8609
Re: RFR: 8283651: nsk/jvmti/SuspendThread/suspendthrd003 may leak native memory
On Mon, 9 May 2022 20:15:21 GMT, Daniel D. Daugherty wrote: > A trivial fix to nsk_jvmti_threadByName() to solve a native memory leak. > > This fix has been stress tested with my > StressWrapper60M_NMT_detail_suspendthrd003.java > test and I did NMT detailed monitoring for the whole 60M run. Total committed > memory has > gone down from 1.17GB to 300MB. Prior to my last rebase, it was down to 239MB. > > I ran the fix thru Mach5 Tier[1-7] with no failures. I just rebased the fix > to the latest jdk/jdk > and Mach5 Tier[1-3] have passed with no failures. Tier[45] are still running. Marked as reviewed by lmesnik (Reviewer). Yes, I agree, the fix is trivial. - PR: https://git.openjdk.java.net/jdk/pull/8609
Re: RFR: 8285739: disable EA when both JVMTI and Preview are enabled
On Mon, 9 May 2022 21:31:12 GMT, Richard Reingruber wrote: > Hi Leonid, > > have you done some testing? > [JDK-8227745](https://bugs.openjdk.java.net/browse/JDK-8227745) came with a > bunch of tests. I wonder how they behave with your change. > > Thanks, Richard. Thank you for looking on this. The EA-related tests passed and still pass when are executed without any additional options. There is a special mode when the test runs main() method in the virtual thread. Test IterateHeapWithEscapeAnalysisEnabled.java failed before the fix and start passing after my fix. It failed because before the fix EscapeBarrier was disabled while EA was enabled and we got incorrect data. Unfortunately, I was unable to run EATests.java in this mode even before the fix. (Not all tests are compatible with this "wrapper" mode) The goal is to restore the same behavior as it was before JDK-8227745 is implemented when virtual threads are enabled. - PR: https://git.openjdk.java.net/jdk/pull/8589
Re: RFR: 8285739: disable EA when both JVMTI and Preview are enabled
On Mon, 9 May 2022 01:36:39 GMT, Leonid Mesnik wrote: > The fix disables EscapeBarrier and EscapeAnalysis when certain JVMTI > capabilities are enabled and --enable-preview. > > It restores the same behavior as it was before > https://bugs.openjdk.java.net/browse/JDK-8227745 "Enable Escape Analysis for > Better Performance in the Presence of JVMTI Agents" is implemented when > Continuations are enabled. Also I run all our testing on the linux-x64-debug to ensure that there are no regressions. - PR: https://git.openjdk.java.net/jdk/pull/8589
Re: RFR: 8285739: disable EA when both JVMTI and Preview are enabled
On Mon, 9 May 2022 01:36:39 GMT, Leonid Mesnik wrote: > The fix disables EscapeBarrier and EscapeAnalysis when certain JVMTI > capabilities are enabled and --enable-preview. > > It restores the same behavior as it was before > https://bugs.openjdk.java.net/browse/JDK-8227745 "Enable Escape Analysis for > Better Performance in the Presence of JVMTI Agents" is implemented when > Continuations are enabled. Hi Leonid, have you done some testing? [JDK-8227745](https://bugs.openjdk.java.net/browse/JDK-8227745) came with a bunch of tests. I wonder how they behave with your change. Thanks, Richard. - PR: https://git.openjdk.java.net/jdk/pull/8589
RFR: 8279358: vmTestbase/nsk/jvmti/scenarios/jni_interception/JI03/ji03t003/TestDescription.java fails with usage tracker
isThreadExpected function checks only some known JFR/Graal threads. This approach requires to keep this function up to date (add other internal threads like usage tracker, loom, etc). To avoid this updated all tests which use it, now the tests filter out all unknown threads. - Commit messages: - fixed all isThreadExpected callers Changes: https://git.openjdk.java.net/jdk/pull/8614/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8614&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279358 Stats: 209 lines in 10 files changed: 131 ins; 40 del; 38 mod Patch: https://git.openjdk.java.net/jdk/pull/8614.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8614/head:pull/8614 PR: https://git.openjdk.java.net/jdk/pull/8614
Re: RFR: 8286438: Add jhsdb jstack processing without --mixed in efh
On Mon, 9 May 2022 20:35:57 GMT, Leonid Mesnik wrote: > The default is required only to set the same depth level in the tree on HTML > page for default and mixed mode. Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8610
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments
On Mon, 9 May 2022 15:56:35 GMT, Adam Sotona wrote: > Please review this patch adding new lint option, **lossy-conversions**, to > javac to warn about type casts in compound assignments with possible lossy > conversions. > > The new lint warning is shown if the type of the right-hand operand of a > compound assignment is not assignment compatible with the type of the > variable. > > The implementation of the warning is based on similar check performed to emit > "possible lossy conversion" compilation error for simple assignments. > > Proposed patch also include complex matrix-style test with positive and > negative test cases of lossy conversions in compound assignments. > > Proposed patch also disables this new lint option in all affected JDK modules > and libraries to allow smooth JDK build. Individual cases to address possibly > lossy conversions warnings in JDK are already addressed in a separate > umbrella issue and its sub-tasks. > > Thanks for your review, > Adam I see there is already a bug filed to address situations found by the new warning in the JDK's libraries (JDK-8286374). As a matter of policy, I recommend the (potential) warnings be addressed in at least the java.base and java.desktop modules before the new warning is enabled. In other words, a priority should be given to keeping java.base and java.desktop compiling successfully with all warnings enabled. - PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8278123: serviceability/dcmd/vm/ClassLoaderStatsTest.java failing with java.lang.AssertionError: Should have a hidden class [v2]
On Thu, 28 Apr 2022 21:58:46 GMT, Leonid Mesnik wrote:
>> The test failed if GC happens somewhere between
>> Class c = Class.forName("TestClass", true, dummyloader);
>> and
>> OutputAnalyzer output = executor.execute("VM.classloader_stats");
>>
>> The fix is to make hc static as Chris proposed.
>>
>> To verfiy fix I add System.gc() before
>> executor.execute("VM.classloader_stats");
>
> Leonid Mesnik has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains three additional
> commits since the last revision:
>
> - comment fixed
> - Merge branch 'master' of https://github.com/openjdk/jdk into 8278123
> - fix
Marked as reviewed by cjplummer (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/8438
Re: RFR: 8285739: disable EA when both JVMTI and Preview are enabled
On Mon, 9 May 2022 01:36:39 GMT, Leonid Mesnik wrote: > The fix disables EscapeBarrier and EscapeAnalysis when certain JVMTI > capabilities are enabled and --enable-preview. > > It restores the same behavior as it was before > https://bugs.openjdk.java.net/browse/JDK-8227745 "Enable Escape Analysis for > Better Performance in the Presence of JVMTI Agents" is implemented when > Continuations are enabled. Good. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8589
Re: RFR: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable [v2]
On Thu, 31 Mar 2022 08:03:23 GMT, Andrey Turbanov wrote:
>> 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 avgt5 1,173 ± 0,026 ns/op
>> integerDouble_isInstance avgt5 0,939 ± 0,038 ns/op
>> integerIntegerDouble_isAssignableFrom avgt5 2,106 ± 0,068 ns/op
>> numberIntegerDouble_isInstance avgt5 1,516 ± 0,046 ns/op
>> integerInteger_isAssignableFromavgt5 1,175 ± 0,029 ns/op
>> integerInteger_isInstance avgt5 0,886 ± 0,017 ns/op
>> numberDouble_isAssignableFrom avgt5 1,172 ± 0,007 ns/op
>> numberDouble_isInstanceavgt5 0,891 ± 0,030 ns/op
>> numberInteger_isAssignableFrom avgt5 1,169 ± 0,014 ns/op
>> numberInteger_isInstance avgt5 0,887 ± 0,016 ns/op
>
> Andrey Turbanov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8280035: Use Class.isInstance instead of Class.isAssignableFrom where
> applicable
> apply suggestion to avoid second Method.invoke call
Marked as reviewed by prr (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/7061
Withdrawn: 8285739: disable EA when both JVMTI and Preview are enabled
On Mon, 9 May 2022 01:36:39 GMT, Leonid Mesnik wrote: > The fix disables EscapeBarrier and EscapeAnalysis when certain JVMTI > capabilities are enabled and --enable-preview. > > It restores the same behavior as it was before > https://bugs.openjdk.java.net/browse/JDK-8227745 "Enable Escape Analysis for > Better Performance in the Presence of JVMTI Agents" is implemented when > Continuations are enabled. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/8589
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments
On Mon, 9 May 2022 15:56:35 GMT, Adam Sotona wrote:
> Please review this patch adding new lint option, **lossy-conversions**, to
> javac to warn about type casts in compound assignments with possible lossy
> conversions.
>
> The new lint warning is shown if the type of the right-hand operand of a
> compound assignment is not assignment compatible with the type of the
> variable.
>
> The implementation of the warning is based on similar check performed to emit
> "possible lossy conversion" compilation error for simple assignments.
>
> Proposed patch also include complex matrix-style test with positive and
> negative test cases of lossy conversions in compound assignments.
>
> Proposed patch also disables this new lint option in all affected JDK modules
> and libraries to allow smooth JDK build. Individual cases to address possibly
> lossy conversions warnings in JDK are already addressed in a separate
> umbrella issue and its sub-tasks.
>
> Thanks for your review,
> Adam
Marked as reviewed by prr (Reviewer).
test/langtools/tools/javac/lint/LossyConversions.java line 131:
> 129: @SuppressWarnings("lossy-conversions")
> 130: public void supressedLossyConversions() {
> 131: byte a = 0;
you might want to spell suppressed correctly.
-
PR: https://git.openjdk.java.net/jdk/pull/8599
RFR: 8285739: disable EA when both JVMTI and Preview are enabled
The fix disables EscapeBarrier and EscapeAnalysis when certain JVMTI capabilities are enabled and --enable-preview. It restores the same behavior as it was before https://bugs.openjdk.java.net/browse/JDK-8227745 "Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents" is implemented when Continuations are enabled. - Commit messages: - 8285739: disable EA when both JVMTI and Preview are enabled Changes: https://git.openjdk.java.net/jdk/pull/8589/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8589&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285739 Stats: 10 lines in 3 files changed: 5 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/8589.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8589/head:pull/8589 PR: https://git.openjdk.java.net/jdk/pull/8589
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments
On Mon, 9 May 2022 15:56:35 GMT, Adam Sotona wrote: > Please review this patch adding new lint option, **lossy-conversions**, to > javac to warn about type casts in compound assignments with possible lossy > conversions. > > The new lint warning is shown if the type of the right-hand operand of a > compound assignment is not assignment compatible with the type of the > variable. > > The implementation of the warning is based on similar check performed to emit > "possible lossy conversion" compilation error for simple assignments. > > Proposed patch also include complex matrix-style test with positive and > negative test cases of lossy conversions in compound assignments. > > Proposed patch also disables this new lint option in all affected JDK modules > and libraries to allow smooth JDK build. Individual cases to address possibly > lossy conversions warnings in JDK are already addressed in a separate > umbrella issue and its sub-tasks. > > Thanks for your review, > Adam Build changes look good. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8599
RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments
Please review this patch adding new lint option, **lossy-conversions**, to javac to warn about type casts in compound assignments with possible lossy conversions. The new lint warning is shown if the type of the right-hand operand of a compound assignment is not assignment compatible with the type of the variable. The implementation of the warning is based on similar check performed to emit "possible lossy conversion" compilation error for simple assignments. Proposed patch also include complex matrix-style test with positive and negative test cases of lossy conversions in compound assignments. Proposed patch also disables this new lint option in all affected JDK modules and libraries to allow smooth JDK build. Individual cases to address possibly lossy conversions warnings in JDK are already addressed in a separate umbrella issue and its sub-tasks. Thanks for your review, Adam - Commit messages: - Merge branch 'openjdk:master' into JDK-8244681 - 8244681: Add a warning for possibly lossy conversion in compound assignments - 8244681: Add a warning for possibly lossy conversion in compound assignments Changes: https://git.openjdk.java.net/jdk/pull/8599/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8599&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8244681 Stats: 449 lines in 26 files changed: 444 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/8599.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8599/head:pull/8599 PR: https://git.openjdk.java.net/jdk/pull/8599
Integrated: 8285794: AsyncGetCallTrace might acquire a lock via JavaThread::thread_from_jni_environment
On Thu, 28 Apr 2022 09:38:30 GMT, Johannes Bechberger wrote: > Calling JavaThread::thread_from_jni_environment for a terminated thread in > AsyncGetCallTrace might cause the acquisition of a lock, making > AsyncGetCallTrace non-signal-safe. > > AsyncGetCallTrace can only be called for the current threads (there are > asserts for that), therefore using JavaThread::current directly and checking > the termination status is semantically equivalent. This pull request has now been integrated. Changeset: d4474b58 Author:Johannes Bechberger Committer: Martin Doerr URL: https://git.openjdk.java.net/jdk/commit/d4474b5816c2ec8daaf1c905b77d8ba4e23c9439 Stats: 7 lines in 1 file changed: 4 ins; 0 del; 3 mod 8285794: AsyncGetCallTrace might acquire a lock via JavaThread::thread_from_jni_environment Reviewed-by: dholmes, mdoerr, jbachorik - PR: https://git.openjdk.java.net/jdk/pull/8446
Re: RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v6]
> Simple rename and some comments update. > > Test: build Albert Mingkun Yang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits: - merge - review - comment - Rework reference type initialization Signed-off-by: Albert Yang - Remove REF_ enum for java.lang.ref.Reference Signed-off-by: Albert Yang - review - ref-rename - Changes: https://git.openjdk.java.net/jdk/pull/8332/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8332&range=05 Stats: 119 lines in 14 files changed: 81 ins; 31 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/8332.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8332/head:pull/8332 PR: https://git.openjdk.java.net/jdk/pull/8332
