Re: RFR 8218167: nsk/jvmti/scenarios/sampling/SP02/sp02t003 fails
Okay, thanks! Serguei On 3/5/19 2:52 PM, Daniil Titov wrote: Hi Serguei, I tested it with different options: -Xcomp and Graal. I also had tier1, tier2 and tier3 tests passed. Bests regards, Daniil On 3/5/19, 2:29 PM, "serguei.spit...@oracle.com" wrote: Hi Daniil, It looks okay. How did you test this fix? Did you run these tests in different compiler modes? Thanks, Serguei On 3/4/19 3:03 PM, Daniil Titov wrote: > Hi Dean, > > You are right, test sp06t003 has the same problem. Please, review a new version of the change that fixes both tests. I checked other tests and no more tests use the this approach with "commonDepth". > > Webrev: http://cr.openjdk.java.net/~dtitov/8218167/webrev.02 > Bug: https://bugs.openjdk.java.net/browse/JDK-8218167 > > Thanks! > --Daniil > > On 3/1/19, 9:14 PM, "serviceability-dev-boun...@openjdk.java.net on behalf of dean.l...@oracle.com" wrote: > > Looks good, but what about sp06t003? Doesn't it have the same problem? > Are there any other tests using similar logic? > > dl > > On 3/1/19 8:33 PM, Daniil Titov wrote: > > Please review the change that fix intermittent failure for test nsk/jvmti/scenarios/sampling/SP02/sp02t003 when running with Graal. > > > > The problem with the test here is that method checkThread() looks for the test method in the top "commonDepth" frames where "commonDepth" is a minimum of "frameCount" (returned by jvmti->GetFrameCount) and "frameStackSize"( returned by jvmti->GetStackTrace). > > > > If a compilation is triggered between these 2 calls then there are cases when "frameCount" is 2, "frameStackSize" is 4, and the frame stack is as the following: > > > > [0] adjustCompilationLevel > > [1] adjustCompilationLevel > > [2] testedMethod > > [3] run > > > > In this case the test looks for the test method only in 2 top frames and fails. > > > > The fix ensures that the test iterates over all frames in the frame stack when looking for the test method. > > > > Webrev: http://cr.openjdk.java.net/~dtitov/8218167/webrev.01 > > Bug: https://bugs.openjdk.java.net/browse/JDK-8218167 > > > > Thanks! > > --Daniil > > > > > > > > >
Re: RFR 8218167: nsk/jvmti/scenarios/sampling/SP02/sp02t003 fails
Hi Serguei, I tested it with different options: -Xcomp and Graal. I also had tier1, tier2 and tier3 tests passed. Bests regards, Daniil On 3/5/19, 2:29 PM, "serguei.spit...@oracle.com" wrote: Hi Daniil, It looks okay. How did you test this fix? Did you run these tests in different compiler modes? Thanks, Serguei On 3/4/19 3:03 PM, Daniil Titov wrote: > Hi Dean, > > You are right, test sp06t003 has the same problem. Please, review a new version of the change that fixes both tests. I checked other tests and no more tests use the this approach with "commonDepth". > > Webrev: http://cr.openjdk.java.net/~dtitov/8218167/webrev.02 > Bug: https://bugs.openjdk.java.net/browse/JDK-8218167 > > Thanks! > --Daniil > > On 3/1/19, 9:14 PM, "serviceability-dev-boun...@openjdk.java.net on behalf of dean.l...@oracle.com" wrote: > > Looks good, but what about sp06t003? Doesn't it have the same problem? > Are there any other tests using similar logic? > > dl > > On 3/1/19 8:33 PM, Daniil Titov wrote: > > Please review the change that fix intermittent failure for test nsk/jvmti/scenarios/sampling/SP02/sp02t003 when running with Graal. > > > > The problem with the test here is that method checkThread() looks for the test method in the top "commonDepth" frames where "commonDepth" is a minimum of "frameCount" (returned by jvmti->GetFrameCount) and "frameStackSize"( returned by jvmti->GetStackTrace). > > > > If a compilation is triggered between these 2 calls then there are cases when "frameCount" is 2, "frameStackSize" is 4, and the frame stack is as the following: > > > > [0] adjustCompilationLevel > > [1] adjustCompilationLevel > > [2] testedMethod > > [3] run > > > > In this case the test looks for the test method only in 2 top frames and fails. > > > > The fix ensures that the test iterates over all frames in the frame stack when looking for the test method. > > > > Webrev: http://cr.openjdk.java.net/~dtitov/8218167/webrev.01 > > Bug: https://bugs.openjdk.java.net/browse/JDK-8218167 > > > > Thanks! > > --Daniil > > > > > > > > >
Re: RFR: 8218464: vmTestbase/nsk/jdi/VirtualMachine/allThreads/allthreads001/TestDescription.java failed
Hi Daniil, The fix looks good. Thanks, Serguei On 3/5/19 1:59 PM, Daniil Titov wrote: Please review a fix for this test that intermittently fails with Graal on. The problem here is that the test does two consequent calls to VirtualMachine.allThreads() and checks that the number of threads returned by these calls is the same. With Graal on there is a chance that a new JVMCI compiler thread could be started between these calls that makes test to fail. The fix temporary suspends the debuggee VM while these two calls to VirtualMachine.allThreads() are made. Webrev: http://cr.openjdk.java.net/~dtitov/8218464/webrev.01 Bug: https://bugs.openjdk.java.net/browse/JDK-8218464 Thanks! --Daniil
Re: RFR:8219721: jcmd from earlier release will hang attaching to VM with JDK-8215622 applied
Hi Lin, It looks good to me too. Thanks, Serguei On 3/4/19 1:13 AM, 臧琳 wrote: Dear All, May I ask your help to review the fix of compatibility issue introduced by JDK-8215622 webrev: http://cr.openjdk.java.net/~xiaofeya/8219721/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8219721 FYI, this webrev is forked from the discussion https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-February/027240.html. Thanks, Lin
Re: RFR 8218167: nsk/jvmti/scenarios/sampling/SP02/sp02t003 fails
Hi Daniil, It looks okay. How did you test this fix? Did you run these tests in different compiler modes? Thanks, Serguei On 3/4/19 3:03 PM, Daniil Titov wrote: Hi Dean, You are right, test sp06t003 has the same problem. Please, review a new version of the change that fixes both tests. I checked other tests and no more tests use the this approach with "commonDepth". Webrev: http://cr.openjdk.java.net/~dtitov/8218167/webrev.02 Bug: https://bugs.openjdk.java.net/browse/JDK-8218167 Thanks! --Daniil On 3/1/19, 9:14 PM, "serviceability-dev-boun...@openjdk.java.net on behalf of dean.l...@oracle.com" wrote: Looks good, but what about sp06t003? Doesn't it have the same problem? Are there any other tests using similar logic? dl On 3/1/19 8:33 PM, Daniil Titov wrote: > Please review the change that fix intermittent failure for test nsk/jvmti/scenarios/sampling/SP02/sp02t003 when running with Graal. > > The problem with the test here is that method checkThread() looks for the test method in the top "commonDepth" frames where "commonDepth" is a minimum of "frameCount" (returned by jvmti->GetFrameCount) and "frameStackSize"( returned by jvmti->GetStackTrace). > > If a compilation is triggered between these 2 calls then there are cases when "frameCount" is 2, "frameStackSize" is 4, and the frame stack is as the following: > > [0] adjustCompilationLevel > [1] adjustCompilationLevel > [2] testedMethod > [3] run > > In this case the test looks for the test method only in 2 top frames and fails. > > The fix ensures that the test iterates over all frames in the frame stack when looking for the test method. > > Webrev: http://cr.openjdk.java.net/~dtitov/8218167/webrev.01 > Bug: https://bugs.openjdk.java.net/browse/JDK-8218167 > > Thanks! > --Daniil > >
RFR: 8218464: vmTestbase/nsk/jdi/VirtualMachine/allThreads/allthreads001/TestDescription.java failed
Please review a fix for this test that intermittently fails with Graal on. The problem here is that the test does two consequent calls to VirtualMachine.allThreads() and checks that the number of threads returned by these calls is the same. With Graal on there is a chance that a new JVMCI compiler thread could be started between these calls that makes test to fail. The fix temporary suspends the debuggee VM while these two calls to VirtualMachine.allThreads() are made. Webrev: http://cr.openjdk.java.net/~dtitov/8218464/webrev.01 Bug: https://bugs.openjdk.java.net/browse/JDK-8218464 Thanks! --Daniil
Re: RFR 8218167: nsk/jvmti/scenarios/sampling/SP02/sp02t003 fails
Thank you, Dean and Chris, for reviewing this change! --Daniil On 3/4/19, 7:00 PM, "Chris Plummer" wrote: Hi Daniil, I think the fix is fine. I was wondering why the test was originally written to use the smaller of the two stack sizes. I noticed that the "suspended == NSK_TRUE" checks seem to defend against test failures that might be introduced with your change (you can only get two different stack sizes when the thread is not suspended). This check was added as part of the fix for JDK-8051349, which was done a few months ago, so if the fix for JDK-8051349 were not in place then your fix would not have worked. thanks, Chris On 3/4/19 3:03 PM, Daniil Titov wrote: > Hi Dean, > > You are right, test sp06t003 has the same problem. Please, review a new version of the change that fixes both tests. I checked other tests and no more tests use the this approach with "commonDepth". > > Webrev: http://cr.openjdk.java.net/~dtitov/8218167/webrev.02 > Bug: https://bugs.openjdk.java.net/browse/JDK-8218167 > > Thanks! > --Daniil > > On 3/1/19, 9:14 PM, "serviceability-dev-boun...@openjdk.java.net on behalf of dean.l...@oracle.com" wrote: > > Looks good, but what about sp06t003? Doesn't it have the same problem? > Are there any other tests using similar logic? > > dl > > On 3/1/19 8:33 PM, Daniil Titov wrote: > > Please review the change that fix intermittent failure for test nsk/jvmti/scenarios/sampling/SP02/sp02t003 when running with Graal. > > > > The problem with the test here is that method checkThread() looks for the test method in the top "commonDepth" frames where "commonDepth" is a minimum of "frameCount" (returned by jvmti->GetFrameCount) and "frameStackSize"( returned by jvmti->GetStackTrace). > > > > If a compilation is triggered between these 2 calls then there are cases when "frameCount" is 2, "frameStackSize" is 4, and the frame stack is as the following: > > > > [0] adjustCompilationLevel > > [1] adjustCompilationLevel > > [2] testedMethod > > [3] run > > > > In this case the test looks for the test method only in 2 top frames and fails. > > > > The fix ensures that the test iterates over all frames in the frame stack when looking for the test method. > > > > Webrev: http://cr.openjdk.java.net/~dtitov/8218167/webrev.01 > > Bug: https://bugs.openjdk.java.net/browse/JDK-8218167 > > > > Thanks! > > --Daniil > > > > > > > > >
Re: RFR(XS) 8220030: JdbStopThreadidTest.java failed due to "Unexpected IO error while writing command 'quit' to jdb stdin stream"
LGTM too. --alex On 03/05/2019 04:47, Gary Adams wrote: This looks OK to me. If a specific return code is expected, then the test should wait until it gets the response it needs. If a specific return is not required, the test should be lenient about what information is required for the test to complete. On 3/4/19, 10:41 PM, Chris Plummer wrote: Hello, Please review the following. Details are in the bug. In short, the way I was checking for the application exit was a bit non-standard and exposed what is probably a bug in the JdbTest shutdown code. I changed the test to conform to the way other test run to exit: https://bugs.openjdk.java.net/browse/JDK-8220030 diff --git a/test/jdk/com/sun/jdi/JdbStopThreadidTest.java b/test/jdk/com/sun/jdi/JdbStopThreadidTest.java --- a/test/jdk/com/sun/jdi/JdbStopThreadidTest.java +++ b/test/jdk/com/sun/jdi/JdbStopThreadidTest.java @@ -32,6 +32,7 @@ * @run main/othervm JdbStopThreadidTest */ +import jdk.test.lib.process.OutputAnalyzer; import lib.jdb.Jdb; import lib.jdb.JdbCommand; import lib.jdb.JdbTest; @@ -138,6 +139,7 @@ jdb.command(JdbCommand.cont().waitForPrompt("Breakpoint hit: \"thread=MYTHREAD-2\", \\S+MyThread.brkMethod", true)); // Continue until the application exits. Once again, hitting a breakpoint will cause // a failure because we are not suppose to hit one. - jdb.command(JdbCommand.cont().waitForPrompt(Jdb.APPLICATION_EXIT, true)); + jdb.contToExit(1); + new OutputAnalyzer(getJdbOutput()).shouldContain(Jdb.APPLICATION_EXIT); thanks, Chris
Re: RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't
Hi Daniel, Latest webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8219585/03/webrev/ It's incorporating your changes wrt. to some timeouts and asserting the expected exit code. Instead of the ProcessThread changes, I'm using the sendMessage() approach. That API is already there. Unfortunately when running both SSL and plain sockets tests, it would randomly fail for me. Even if I choose different sets of ports for plain/ssl. So I've taken a different route of randomly selecting SSL or plain. Overall, this should give reasonable coverage for both (plain and SSL). This made test stability improve a lot on my Linux x86_64 machine. Ran for 100 iterations without failure. I'll run this through jdk/submit now. Could you run this through your test system too, please? Would you be OK with getting this patch pushed once we'd have positive results for both? Thanks, Severin On Fri, 2019-03-01 at 15:08 +, Daniel Fuchs wrote: > Hi Severin, > > On 28/02/2019 15:47, Severin Gehwolf wrote: > > Yes, thanks for looking at this Daniel. That was my determination as > > well. However, even if we do all of the above, and add a test config > > with /etc/hosts mapping a non-loopback address to "localhost" it would > > break other tests. E.g. this one: > > java/net/InetAddress/GetLocalHostWithSM.java > > > > So I'd have to explore whether your suggestion with > > InetAddress.getLocalHost() is viable. It seems promising. > > > > I'll keep you posted. > > Thanks for keeping the investigation going! > > For what it's worth this is what I have been experimenting with: > http://cr.openjdk.java.net/~dfuchs/experiment-8219585/experiment.00/ > > It's only a POC and obviously need more cleaning. > Maybe some of the arbitrary timeouts in the test could be scaled > in accordance to timeout-factor (I think there's an adjustTimeout(long) > function somewhere in the test libs that does that). > > I ran it 50 times in our test system - and it passed on all platforms, > so there's yet hope :-) > > hope this helps, > > -- daniel > >
Re: RFR:8219721: jcmd from earlier release will hang attaching to VM with JDK-8215622 applied
Looks good. Yasumasa (ysuenaga) On 2019/03/04 18:13, zangl...@jd.com wrote: Dear All, May I ask your help to review the fix of compatibility issue introduced by JDK-8215622 webrev: http://cr.openjdk.java.net/~xiaofeya/8219721/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8219721 FYI, this webrev is forked from the discussion https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-February/027240.html. Thanks, Lin
Re: RFR(XS) 8220030: JdbStopThreadidTest.java failed due to "Unexpected IO error while writing command 'quit' to jdb stdin stream"
This looks OK to me. If a specific return code is expected, then the test should wait until it gets the response it needs. If a specific return is not required, the test should be lenient about what information is required for the test to complete. On 3/4/19, 10:41 PM, Chris Plummer wrote: Hello, Please review the following. Details are in the bug. In short, the way I was checking for the application exit was a bit non-standard and exposed what is probably a bug in the JdbTest shutdown code. I changed the test to conform to the way other test run to exit: https://bugs.openjdk.java.net/browse/JDK-8220030 diff --git a/test/jdk/com/sun/jdi/JdbStopThreadidTest.java b/test/jdk/com/sun/jdi/JdbStopThreadidTest.java --- a/test/jdk/com/sun/jdi/JdbStopThreadidTest.java +++ b/test/jdk/com/sun/jdi/JdbStopThreadidTest.java @@ -32,6 +32,7 @@ * @run main/othervm JdbStopThreadidTest */ +import jdk.test.lib.process.OutputAnalyzer; import lib.jdb.Jdb; import lib.jdb.JdbCommand; import lib.jdb.JdbTest; @@ -138,6 +139,7 @@ jdb.command(JdbCommand.cont().waitForPrompt("Breakpoint hit: \"thread=MYTHREAD-2\", \\S+MyThread.brkMethod", true)); // Continue until the application exits. Once again, hitting a breakpoint will cause // a failure because we are not suppose to hit one. - jdb.command(JdbCommand.cont().waitForPrompt(Jdb.APPLICATION_EXIT, true)); +jdb.contToExit(1); +new OutputAnalyzer(getJdbOutput()).shouldContain(Jdb.APPLICATION_EXIT); thanks, Chris
RE: RFR:8219721: jcmd from earlier release will hang attaching to VM with JDK-8215622 applied
Hi David, Thanks a lot for your review. Hi All, May I get more review about this patch, it is a trivial patch to fix compatibility issue. Thanks, Lin > -Original Message- > From: David Holmes > Sent: Tuesday, March 5, 2019 2:12 PM > To: 臧琳 ; serviceability-dev@openjdk.java.net > serviceability-dev@openjdk.java.net > Subject: Re: RFR:8219721: jcmd from earlier release will hang attaching to VM > with JDK-8215622 applied > > Hi Lin, > > On 4/03/2019 7:13 pm, 臧琳 wrote: > > Dear All, > > May I ask your help to review the fix of compatibility issue introduced > > by > JDK-8215622 > > webrev: http://cr.openjdk.java.net/~xiaofeya/8219721/webrev.01/ > > Bug: https://bugs.openjdk.java.net/browse/JDK-8219721 > > > > FYI, this webrev is forked from the discussion > https://mail.openjdk.java.net/pipermail/serviceability-dev/2019- > February/027240.html. > > This looks good to me. I can confirm that it fixes the problem of tools from > earlier releases hanging. > > Thanks, > David > > > Thanks, > > Lin > >