Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr [v3]
On Fri, 10 Dec 2021 21:12:45 GMT, Chris Plummer wrote: >> The test searches for "JShellToolProvider" in the main thread's stack trace, >> which is pulled from an SA heap dump. Typically the main thread is blocked >> in Object.wait(), so SA can determine its stack trace. However, the wait has >> a 100ms timeout, so the thread does periodically wake up and does a few >> things before waiting again. If SA tries to get the stack trace while the >> thread is active, it may not be able to, and this causes the test to fail. I >> determined this only happens about 1 in 5000-1 runs. So as a fix I'm >> allowing the test to do one retry. That should make it extremely unlikely >> that we ever see this failure again. I also bumped up the amount of time the >> test waits before doing the heap dump from 2 seconds to 4 just to make >> absolutely sure the main thread is fully started before doing the heap dump. > > Chris Plummer has updated the pull request incrementally with one additional > commit since the last revision: > > Fix allowRetry reference. Marked as reviewed by sspitsyn (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6795
Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr [v3]
On Fri, 10 Dec 2021 21:12:45 GMT, Chris Plummer wrote: >> The test searches for "JShellToolProvider" in the main thread's stack trace, >> which is pulled from an SA heap dump. Typically the main thread is blocked >> in Object.wait(), so SA can determine its stack trace. However, the wait has >> a 100ms timeout, so the thread does periodically wake up and does a few >> things before waiting again. If SA tries to get the stack trace while the >> thread is active, it may not be able to, and this causes the test to fail. I >> determined this only happens about 1 in 5000-1 runs. So as a fix I'm >> allowing the test to do one retry. That should make it extremely unlikely >> that we ever see this failure again. I also bumped up the amount of time the >> test waits before doing the heap dump from 2 seconds to 4 just to make >> absolutely sure the main thread is fully started before doing the heap dump. > > Chris Plummer has updated the pull request incrementally with one additional > commit since the last revision: > > Fix allowRetry reference. Marked as reviewed by amenkov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6795
Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr [v2]
On Fri, 10 Dec 2021 21:05:38 GMT, Alex Menkov wrote:
>> Chris Plummer has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Renamed isRetry to allowRetry.
>
> test/jdk/sun/tools/jhsdb/JShellHeapDumpTest.java line 112:
>
>> 110: // if that happens. This failure is so rare that this
>> should be enough to make it
>> 111: // extremely unlikely that we ever see this test fail
>> again for this reason.
>> 112: if (allowRetry) {
>
> Looks like this should be !allowRetry
Yeah, I was just looking at the diff an noticed that, but you beat me to it.
Will push another commit to fix.
-
PR: https://git.openjdk.java.net/jdk/pull/6795
Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr [v2]
On Fri, 10 Dec 2021 20:30:41 GMT, Chris Plummer wrote:
>> The test searches for "JShellToolProvider" in the main thread's stack trace,
>> which is pulled from an SA heap dump. Typically the main thread is blocked
>> in Object.wait(), so SA can determine its stack trace. However, the wait has
>> a 100ms timeout, so the thread does periodically wake up and does a few
>> things before waiting again. If SA tries to get the stack trace while the
>> thread is active, it may not be able to, and this causes the test to fail. I
>> determined this only happens about 1 in 5000-1 runs. So as a fix I'm
>> allowing the test to do one retry. That should make it extremely unlikely
>> that we ever see this failure again. I also bumped up the amount of time the
>> test waits before doing the heap dump from 2 seconds to 4 just to make
>> absolutely sure the main thread is fully started before doing the heap dump.
>
> Chris Plummer has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Renamed isRetry to allowRetry.
Changes requested by amenkov (Reviewer).
test/jdk/sun/tools/jhsdb/JShellHeapDumpTest.java line 112:
> 110: // if that happens. This failure is so rare that this
> should be enough to make it
> 111: // extremely unlikely that we ever see this test fail
> again for this reason.
> 112: if (allowRetry) {
Looks like this should be !allowRetry
-
PR: https://git.openjdk.java.net/jdk/pull/6795
Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr [v3]
> The test searches for "JShellToolProvider" in the main thread's stack trace, > which is pulled from an SA heap dump. Typically the main thread is blocked in > Object.wait(), so SA can determine its stack trace. However, the wait has a > 100ms timeout, so the thread does periodically wake up and does a few things > before waiting again. If SA tries to get the stack trace while the thread is > active, it may not be able to, and this causes the test to fail. I determined > this only happens about 1 in 5000-1 runs. So as a fix I'm allowing the > test to do one retry. That should make it extremely unlikely that we ever see > this failure again. I also bumped up the amount of time the test waits before > doing the heap dump from 2 seconds to 4 just to make absolutely sure the main > thread is fully started before doing the heap dump. Chris Plummer has updated the pull request incrementally with one additional commit since the last revision: Fix allowRetry reference. - Changes: - all: https://git.openjdk.java.net/jdk/pull/6795/files - new: https://git.openjdk.java.net/jdk/pull/6795/files/aaa915b3..8be1552a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6795=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6795=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6795.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6795/head:pull/6795 PR: https://git.openjdk.java.net/jdk/pull/6795
Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr [v2]
On Fri, 10 Dec 2021 12:22:22 GMT, Serguei Spitsyn wrote: > Hi Chris, > It looks good in general. I feel the parameter 'retry' is a little bit > confusing. It is possible to rename it to some to 'retriedOnce'. Another > approach would be to name it 'allowRetry' and revert the meaning of boolean > value. > I leave it up to you to keep the original approach or make any change. > Thanks, > Serguei Ok. I changed it to allowRetry. - PR: https://git.openjdk.java.net/jdk/pull/6795
Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr [v2]
> The test searches for "JShellToolProvider" in the main thread's stack trace, > which is pulled from an SA heap dump. Typically the main thread is blocked in > Object.wait(), so SA can determine its stack trace. However, the wait has a > 100ms timeout, so the thread does periodically wake up and does a few things > before waiting again. If SA tries to get the stack trace while the thread is > active, it may not be able to, and this causes the test to fail. I determined > this only happens about 1 in 5000-1 runs. So as a fix I'm allowing the > test to do one retry. That should make it extremely unlikely that we ever see > this failure again. I also bumped up the amount of time the test waits before > doing the heap dump from 2 seconds to 4 just to make absolutely sure the main > thread is fully started before doing the heap dump. Chris Plummer has updated the pull request incrementally with one additional commit since the last revision: Renamed isRetry to allowRetry. - Changes: - all: https://git.openjdk.java.net/jdk/pull/6795/files - new: https://git.openjdk.java.net/jdk/pull/6795/files/65e14883..aaa915b3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6795=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6795=00-01 Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/6795.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6795/head:pull/6795 PR: https://git.openjdk.java.net/jdk/pull/6795
Re: RFR: 8277481: Obsolete seldom used CDS flags [v2]
On Fri, 10 Dec 2021 19:37:31 GMT, Calvin Cheung wrote:
>> Harold Seigel has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fix print_debug() message
>
> src/jdk.hotspot.agent/share/native/libsaproc/ps_core_common.c line 303:
>
>> 301: useSharedSpacesAddr = lookup_symbol(ph, jvm_name,
>> USE_SHARED_SPACES_SYM);
>> 302: if (useSharedSpacesAddr == 0) {
>> 303: print_debug("can't lookup 'UseSharedSpaces' symbol\n");
>
> Maybe the `print_debug` at line 311 should also be updated from "flag" to
> "symbol"?
fixed. Thanks for pointing it out.
-
PR: https://git.openjdk.java.net/jdk/pull/6800
Re: RFR: 8277481: Obsolete seldom used CDS flags [v2]
> Please review this change to obsolete deprecated CDS options UseSharedSpaces, > RequireSharedSpaces, DynamicDumpSharedSpaces, and DumpSharedSpaces. The > change was tested by running Mach5 tiers 1-2 on Linux, Mac OS, and Windows > and Mach5 tiers 3-5 on Linux x64 and Windows x64. > > The use of UseSharedSpaces in ps_core_common.c was tested on Mac OS x64 by > temporarily removing serviceability/sa/ClhsdbPmap.java#core from the problem > list. > > Thanks! Harold Harold Seigel has updated the pull request incrementally with one additional commit since the last revision: fix print_debug() message - Changes: - all: https://git.openjdk.java.net/jdk/pull/6800/files - new: https://git.openjdk.java.net/jdk/pull/6800/files/3f6c6dee..601a678f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6800=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6800=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6800.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6800/head:pull/6800 PR: https://git.openjdk.java.net/jdk/pull/6800
Re: RFR: 8277481: Obsolete seldom used CDS flags
On Fri, 10 Dec 2021 15:01:29 GMT, Harold Seigel wrote:
> Please review this change to obsolete deprecated CDS options UseSharedSpaces,
> RequireSharedSpaces, DynamicDumpSharedSpaces, and DumpSharedSpaces. The
> change was tested by running Mach5 tiers 1-2 on Linux, Mac OS, and Windows
> and Mach5 tiers 3-5 on Linux x64 and Windows x64.
>
> The use of UseSharedSpaces in ps_core_common.c was tested on Mac OS x64 by
> temporarily removing serviceability/sa/ClhsdbPmap.java#core from the problem
> list.
>
> Thanks! Harold
Looks good. Just one nit.
src/jdk.hotspot.agent/share/native/libsaproc/ps_core_common.c line 303:
> 301: useSharedSpacesAddr = lookup_symbol(ph, jvm_name,
> USE_SHARED_SPACES_SYM);
> 302: if (useSharedSpacesAddr == 0) {
> 303: print_debug("can't lookup 'UseSharedSpaces' symbol\n");
Maybe the `print_debug` at line 311 should also be updated from "flag" to
"symbol"?
-
Marked as reviewed by ccheung (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6800
Re: RFR: 8277481: Obsolete seldom used CDS flags
On Fri, 10 Dec 2021 15:01:29 GMT, Harold Seigel wrote: > Please review this change to obsolete deprecated CDS options UseSharedSpaces, > RequireSharedSpaces, DynamicDumpSharedSpaces, and DumpSharedSpaces. The > change was tested by running Mach5 tiers 1-2 on Linux, Mac OS, and Windows > and Mach5 tiers 3-5 on Linux x64 and Windows x64. > > The use of UseSharedSpaces in ps_core_common.c was tested on Mac OS x64 by > temporarily removing serviceability/sa/ClhsdbPmap.java#core from the problem > list. > > Thanks! Harold Looks good to me. Thanks for fixing this! - Marked as reviewed by iklam (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6800
Integrated: Merge jdk18
On Fri, 10 Dec 2021 17:51:31 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 18 -> JDK 19 This pull request has now been integrated. Changeset: 61736f81 Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/61736f81fb4a20375c83d59e2b37a00aafb11107 Stats: 1142 lines in 30 files changed: 688 ins; 155 del; 299 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/6802
RFR: Merge jdk18
Forwardport JDK 18 -> JDK 19 - Commit messages: - Merge - 8277621: ARM32: multiple fastdebug failures with "bad AD file" after JDK-8276162 - 8278538: Test langtools/jdk/javadoc/tool/CheckManPageOptions.java fails after the manpage was updated - 8273179: Update nroff pages in JDK 18 before RC The merge commit only contains trivial merges, so no merge-specific webrevs have been generated. Changes: https://git.openjdk.java.net/jdk/pull/6802/files Stats: 1142 lines in 30 files changed: 688 ins; 155 del; 299 mod Patch: https://git.openjdk.java.net/jdk/pull/6802.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6802/head:pull/6802 PR: https://git.openjdk.java.net/jdk/pull/6802
RFR: 8277481: Obsolete seldom used CDS flags
Please review this change to obsolete deprecated CDS options UseSharedSpaces, RequireSharedSpaces, DynamicDumpSharedSpaces, and DumpSharedSpaces. The change was tested by running Mach5 tiers 1-2 on Linux, Mac OS, and Windows and Mach5 tiers 3-5 on Linux x64 and Windows x64. The use of UseSharedSpaces in ps_core_common.c was tested on Mac OS x64 by temporarily removing serviceability/sa/ClhsdbPmap.java#core from the problem list. Thanks! Harold - Commit messages: - fix typo - 8277481: Obsolete seldom used CDS flags Changes: https://git.openjdk.java.net/jdk/pull/6800/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6800=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277481 Stats: 151 lines in 13 files changed: 22 ins; 94 del; 35 mod Patch: https://git.openjdk.java.net/jdk/pull/6800.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6800/head:pull/6800 PR: https://git.openjdk.java.net/jdk/pull/6800
Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr
On Fri, 10 Dec 2021 07:00:10 GMT, Chris Plummer wrote: > The test searches for "JShellToolProvider" in the main thread's stack trace, > which is pulled from an SA heap dump. Typically the main thread is blocked in > Object.wait(), so SA can determine its stack trace. However, the wait has a > 100ms timeout, so the thread does periodically wake up and does a few things > before waiting again. If SA tries to get the stack trace while the thread is > active, it may not be able to, and this causes the test to fail. I determined > this only happens about 1 in 5000-1 runs. So as a fix I'm allowing the > test to do one retry. That should make it extremely unlikely that we ever see > this failure again. I also bumped up the amount of time the test waits before > doing the heap dump from 2 seconds to 4 just to make absolutely sure the main > thread is fully started before doing the heap dump. Hi Chris, It looks good in general. I feel the parameter 'retry' is a little bit confusing. It is possible to rename it to some to 'retriedOnce'. Another approach would be to name it 'allowRetry' and revert the meaning of boolean value. I leave it up to you to keep the original approach or make any change. Thanks, Serguei - Marked as reviewed by sspitsyn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6795
Re: RFR: 8269556: sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr
On Fri, 10 Dec 2021 07:00:10 GMT, Chris Plummer wrote: > The test searches for "JShellToolProvider" in the main thread's stack trace, > which is pulled from an SA heap dump. Typically the main thread is blocked in > Object.wait(), so SA can determine its stack trace. However, the wait has a > 100ms timeout, so the thread does periodically wake up and does a few things > before waiting again. If SA tries to get the stack trace while the thread is > active, it may not be able to, and this causes the test to fail. I determined > this only happens about 1 in 5000-1 runs. So as a fix I'm allowing the > test to do one retry. That should make it extremely unlikely that we ever see > this failure again. I also bumped up the amount of time the test waits before > doing the heap dump from 2 seconds to 4 just to make absolutely sure the main > thread is fully started before doing the heap dump. Looks good. - Marked as reviewed by kevinw (Committer). PR: https://git.openjdk.java.net/jdk/pull/6795
