On 8/15/18 8:46 PM, Alex Menkov wrote:
Hi Dan,
Thank you for the review.
See replies inline.
On 08/15/2018 17:25, Daniel D. Daugherty wrote:
On 8/15/18 6:42 PM, Alex Menkov wrote:
Hi all,
please review a fix for
https://bugs.openjdk.java.net/browse/JDK-8209517
webrev:
http://cr.openjdk.java.net/~amenkov/sh2java/step1_regression/webrev/
test/jdk/com/sun/jdi/BreakpointWithFullGC.java
No comments.
test/jdk/com/sun/jdi/lib/jdb/Jdb.java
L101: 30, TimeUnit.SECONDS);
I'm assuming this is a 30 second timeout for getting the
initial "Listening for transport..." message. Keep in mind
that this may need to scale with timeoutFactor. I have seen
runs with -Xcomp take several minutes here...
L168: waitFor(10, TimeUnit.SECONDS);
L178: debuggee.waitFor(10, TimeUnit.SECONDS);
10 seconds may not be enough on a loaded machine with
lots of parallel processes. Keep in mind that this
may need to scale with timeoutFactor.
Good point.
I'll think how to handle this properly and will update timeouts in one
of the subsequent fixes.
Agreed. I should have been more clear. We'll keep an eye open
timeouts with this test and if they happen, we should tweak
the timeout value algorithm.
I'd prefer to push this fix as soon as I get 2nd approval (as it
produces noise in CI).
Very much agreed. These failures are happening in every JDK12
tier3 CI job set (for some reason).
test/jdk/com/sun/jdi/lib/jdb/JdbCommand.java
No comments.
test/jdk/com/sun/jdi/lib/jdb/JdbTest.java
No comments.
Looks good to me. Is BreakpointWithFullGC.java the only test
that uses the com/sun/jdi/lib/jdb/*.java library code? That
would surprise me, but I haven't been in this test code for
a very long time.
com/sun/jdi/lib/jdb classes were introduced to be used by tests
converted from com/sun/jdi/*.sh
(https://bugs.openjdk.java.net/browse/JDK-8201652)
For now 2 tests are converted:
test/jdk/com/sun/jdi/BreakpointWithFullGC.java
test/jdk/com/sun/jdi/ArrayLengthDumpTest.java
ArrayLengthDumpTest.java does not require to be updated (works fine
with updated lib/jdb)
Thanks for closing the loop on this.
Dan
--alex
Thumbs up!
Dan
Cause of the BreakpointWithFullGC failures is a mess of jdb and
debuggee outputs (the test runs debuggee by using CommandLineLaunch
connector, so jdb redirects debuggee stdout to its own stdout).
To solve it test framework was updated to launch debuggee first
(redirecting its output) and then connecting jdb to existing process.
The approach allow to drop "simple prompt" logic (jdb mode when
debugee is not yet running).
Mach5 passed 400 runs ("4 std platforms" x "--test-repeat 100")
without any issues.
--alex
