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. I'd prefer to push this fix as soon as I get
2nd approval (as it produces noise in CI).
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)
--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
