Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Wed, 15 Mar 2023 00:34:00 GMT, Alex Menkov wrote: >> The change: >> - updates UniqueVtableTest to follow standard SA way - attach to target from >> subprocess and use SATestUtils.addPrivilegesIfNeeded for the subprocess; >> - updates several tests in the same directory to resolve >> NoClassDefFoundError failures; It's known JTReg issue that "@build" actions >> for part of used shared classes may cause intermittent NoClassDefFoundError >> in other tests which use the same shared library classpath. >> >> Tested: 100 runs on all platforms, no failures > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > feedback Sigh... And again we have the situation where some folks are adding `@build` directives and other folks are removing `@build` directives. Another recent PR added library build directives: https://github.com/openjdk/jdk/pull/13085 based on quoted guidance from the JTREG documentation. This mess is related to: [CODETOOLS-7902847](https://bugs.openjdk.org/browse/CODETOOLS-7902847) Class directory of a test case should not be used to compile a library and NoClassDefFoundErrors show up when doing parallel execution of tests where more than one test uses the "offending" library. We really, really need @jonathan-gibbons to chime in on review threads like these. - PR Comment: https://git.openjdk.org/jdk/pull/13030#issuecomment-1476684726
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Wed, 15 Mar 2023 00:34:00 GMT, Alex Menkov wrote: >> The change: >> - updates UniqueVtableTest to follow standard SA way - attach to target from >> subprocess and use SATestUtils.addPrivilegesIfNeeded for the subprocess; >> - updates several tests in the same directory to resolve >> NoClassDefFoundError failures; It's known JTReg issue that "@build" actions >> for part of used shared classes may cause intermittent NoClassDefFoundError >> in other tests which use the same shared library classpath. >> >> Tested: 100 runs on all platforms, no failures > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > feedback Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.org/jdk/pull/13030
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Wed, 15 Mar 2023 00:34:00 GMT, Alex Menkov wrote: >> The change: >> - updates UniqueVtableTest to follow standard SA way - attach to target from >> subprocess and use SATestUtils.addPrivilegesIfNeeded for the subprocess; >> - updates several tests in the same directory to resolve >> NoClassDefFoundError failures; It's known JTReg issue that "@build" actions >> for part of used shared classes may cause intermittent NoClassDefFoundError >> in other tests which use the same shared library classpath. >> >> Tested: 100 runs on all platforms, no failures > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > feedback Marked as reviewed by sspitsyn (Reviewer). - PR: https://git.openjdk.org/jdk/pull/13030
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Wed, 15 Mar 2023 20:02:11 GMT, Alex Menkov wrote: > It seems to me that it's much simpler to remove build action from 4 tests in > the directory than add it for other 55 True. Sadly we keep getting bitten over and over by CODETOOLS-7902847. Sometimes the "fix" is to remove build directives (in Hotspot we switched from adding to removing back in 2017) and sometimes it is to add them (there have been some recent fixes that have done this - including by me). - PR: https://git.openjdk.org/jdk/pull/13030
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Wed, 15 Mar 2023 18:50:25 GMT, Serguei Spitsyn wrote: > This looks pretty good. How did you test the fix? Does it never fail with > your fix? Thanks, Serguei I run test jobs for "serviceability/sa" 100 times on all platforms, no failures (neither attach timeout nor NoClassDefFoundError) - PR: https://git.openjdk.org/jdk/pull/13030
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Wed, 15 Mar 2023 02:41:18 GMT, David Holmes wrote: > Not sure removing the build directives was the right way to go. As per the > jtreg tag guide: > > > A test that relies upon library classes should contain appropriate @build > > directives to ensure that the classes will be compiled. It is strongly > > recommended that tests do not rely on the use of implicit compilation by > > the Java compiler. > > so the problem is likely caused by missing build directives in the test(s) > that fails. This is long standing issue with NoClassDefFoundError for library classes. As far as I got from reading similar issues there was a try to add build directive for failing tests, but other tests from the same directory started to fail with NoClassDefFoundError later, and now most of the test have no build action for libraries. It seems to me that it's much simpler to remove build action from 4 tests in the directory than add it for other 55 - PR: https://git.openjdk.org/jdk/pull/13030
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Wed, 15 Mar 2023 00:34:00 GMT, Alex Menkov wrote: >> The change: >> - updates UniqueVtableTest to follow standard SA way - attach to target from >> subprocess and use SATestUtils.addPrivilegesIfNeeded for the subprocess; >> - updates several tests in the same directory to resolve >> NoClassDefFoundError failures; It's known JTReg issue that "@build" actions >> for part of used shared classes may cause intermittent NoClassDefFoundError >> in other tests which use the same shared library classpath. >> >> Tested: 100 runs on all platforms, no failures > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > feedback This looks pretty good. How did you test the fix? Does it never fail with your fix? Thanks, Serguei - PR: https://git.openjdk.org/jdk/pull/13030
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Wed, 15 Mar 2023 00:34:00 GMT, Alex Menkov wrote: >> The change: >> - updates UniqueVtableTest to follow standard SA way - attach to target from >> subprocess and use SATestUtils.addPrivilegesIfNeeded for the subprocess; >> - updates several tests in the same directory to resolve >> NoClassDefFoundError failures; It's known JTReg issue that "@build" actions >> for part of used shared classes may cause intermittent NoClassDefFoundError >> in other tests which use the same shared library classpath. >> >> Tested: 100 runs on all platforms, no failures > > Alex Menkov has updated the pull request incrementally with one additional > commit since the last revision: > > feedback Not sure removing the build directives was the right way to go. As per the jtreg tag guide: > A test that relies upon library classes should contain appropriate @build > directives to ensure that the classes will be compiled. It is strongly > recommended that tests do not rely on the use of implicit compilation by the > Java compiler. so the problem is likely caused by missing build directives in the test(s) that fails. - PR: https://git.openjdk.org/jdk/pull/13030
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Wed, 15 Mar 2023 00:34:32 GMT, Alex Menkov wrote: >> "else" part is a sub-process. >> As far as I understand it SATestUtils.skipIfCannotAttach can be skipped for >> "else", but it's needed for main process. > > Added comment. > Left SATestUtils.skipIfCannotAttach as is (this is consistent with other SA > tests) > "else" part is a sub-process. Ok, I read it backwards. Part of the reason I asked for comments. :) - PR: https://git.openjdk.org/jdk/pull/13030
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Tue, 14 Mar 2023 23:41:47 GMT, Alex Menkov wrote:
>> test/hotspot/jtreg/serviceability/sa/UniqueVtableTest.java line 195:
>>
>>> 193: } else {
>>> 194: runTest(Long.parseLong(args[0]));
>>> 195: }
>>
>> Could use some comments here. Also, I think `SATestUtils.skipIfCannotAttach`
>> is only needed for the `else` part.
>
> "else" part is a sub-process.
> As far as I understand it SATestUtils.skipIfCannotAttach can be skipped for
> "else", but it's needed for main process.
Added comment.
Left SATestUtils.skipIfCannotAttach as is (this is consistent with other SA
tests)
-
PR: https://git.openjdk.org/jdk/pull/13030
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
On Tue, 14 Mar 2023 22:50:32 GMT, Chris Plummer wrote: >> Alex Menkov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> feedback > > test/hotspot/jtreg/serviceability/sa/UniqueVtableTest.java line 158: > >> 156: Long.toString(lingeredAppPid)); >> 157: SATestUtils.addPrivilegesIfNeeded(processBuilder); >> 158: OutputAnalyzer SAOutput = >> ProcessTools.executeProcess(processBuilder); > > `SAOutput`: local variables should start with lower case. Fixed - PR: https://git.openjdk.org/jdk/pull/13030
Re: RFR: 8303921: serviceability/sa/UniqueVtableTest.java timed out [v2]
> The change: > - updates UniqueVtableTest to follow standard SA way - attach to target from > subprocess and use SATestUtils.addPrivilegesIfNeeded for the subprocess; > - updates several tests in the same directory to resolve NoClassDefFoundError > failures; It's known JTReg issue that "@build" actions for part of used > shared classes may cause intermittent NoClassDefFoundError in other tests > which use the same shared library classpath. > > Tested: 100 runs on all platforms, no failures Alex Menkov has updated the pull request incrementally with one additional commit since the last revision: feedback - Changes: - all: https://git.openjdk.org/jdk/pull/13030/files - new: https://git.openjdk.org/jdk/pull/13030/files/69cc6dae..0e8573d8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=13030=01 - incr: https://webrevs.openjdk.org/?repo=jdk=13030=00-01 Stats: 5 lines in 1 file changed: 2 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/13030.diff Fetch: git fetch https://git.openjdk.org/jdk pull/13030/head:pull/13030 PR: https://git.openjdk.org/jdk/pull/13030
