Hi Chris,
I've replied with a thumbs up on another thread.
Thanks,
Serguei
On 6/3/15 4:23 PM, Chris Plummer wrote:
Hi Serguei,
I'll take care of the rename. Can I also put you down for the
ProcessTool.java changes, which are officially being reviewed on
another thread:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-June/033892.html
thanks,
Chris
On 6/3/15 1:40 AM, serguei.spit...@oracle.com wrote:
Chris,
It looks good in general.
I'd suggest to rename the folder:
|| test/com/sun/jdi/CDSJDITests
to:
test/com/sun/jdi/cds
There is no need to spell "JDI" as it is already a sub-folder of the
com/sun/jdi
and there is no need to spell "Tests" too as it is in the test repo.
Also, all the folders are normally named in the lower case.
Thanks,
Serguei
On 6/2/15 8:25 PM, Chris Plummer wrote:
Ok, I'm going to keep this as one webrev, but I did create
JDK-8081771 for the ProcessTool.java changes:
https://bugs.openjdk.java.net/browse/JDK-8081771
I will commit ProcessTool.java under JDK-8081771, and the rest of
the changes under JDK-8054386. Both will then be pushed together. I
also started a new thread for the review of JDK-8081771:
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2015-June/014930.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-June/033892.html
thanks,
Chris
On 6/2/15 11:55 AM, Chris Plummer wrote:
I'm going to have to separate out the ProcessTool.java changes into
a separate changeset (and CR). In the meantime, feel free to review
what I have below. The code won't be changing at all when I
separate out the ProcessTool.java changes.
thanks,
Chris
On 6/2/15 12:36 AM, Chris Plummer wrote:
[Adding core-libs-dev@openjdk.java.net since this update includes
changes to jdk/test library code]
Please review the updated webrev:
Webrev: http://cr.openjdk.java.net/~cjplummer/8054386/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8054386
There were concerns about the new hotspot tests referencing jdk
tests. One concern was that if the jdk tests change, they could
break the hotspot tests, and this might initially go undetected.
The other concern is that if the jdk and hotspot tests are placed
in separate test bundles, then it would not be possible to run the
hotspot tests.
Because of these concerns, I moved the tests that were in
hotspot/test/runtime/CDSJDITests to instead be in
jdk/test/com/sun/jdi/CDSJDITests. There was a slight renaming of
the tests in the process. Also, I had to update the jdk version of
ProcessTool.java to include the createJavaProcessBuilder() variant
that is in the hotspot version of ProcessTool.java.
Lastly, in CDSJITTest.java I changed:
OutputAnalyzer output = new OutputAnalyzer(pb.start());
to instead be:
OutputAnalyzer output = ProcessTools.executeProcess(pb);
I had to do this since the jdk version of the OutputAnalyzer
constructor is not public. The 1st version is what is commonly
used in hostspot tests, and the 2nd version is what is commonly
used in jdk tests. I decided to adopt the jdk way rather than make
the OutputAnalyzer constructors public, although this will
probably happen eventually when the two versions are unified.
thanks,
Chris
On 5/19/15 7:25 AM, Chris Plummer wrote:
Hi,
Please review the following changes for allowing java debugging
when CDS is enabled.
Webrev:http://cr.openjdk.java.net/~cjplummer/8054386/webrev.01/
Bug:https://bugs.openjdk.java.net/browse/JDK-8054386
The VM changes are simple. I removed the check that prevents
debugging with CDS enabled, and added logic that will map the CDS
archive RW when debugging is enabled.
The tests are a bit more complex. There are a bunch of existing
JDI tests for testing debugging support. Rather than start from
scratch or clone them, I instead just wrote wrapper tests that
put the relevant JDI test classes in the archive, and then invoke
the JDI test. I did this for 3 of the JDI tests. If you feel
there are others that would be good candidates, I'd be happy to
add them. I'm looking for ones that would result in modification
of the RO class metadata, such as setting a breakpoint (for which
I already added two tests).
Testing done:
-Using JPRT to run the new jtreg tests on all platforms.
-Using JPRT to run all jtreg runtime tests on linux x86 and x_64.
-Regular JPRT "-testset hotspot" run
-Putting the JCK JVMTI tests in the archive and then running them.
-Putting the nsk jdb, jdwp, jvmti, and jdi tests in the archive
and then running them.
-Putting a simple test class in the archive and then setting a
breakpoint on it using jdb
Some of the above testing resulted in the discovery of bugs that
still need to be addressed: JDK-8078644, JDK-8078730, and
JDK-8079181.
I also verified that without the change to map the archive RW,
the above testing resulted in a SEGV, which is what you would
expect (and actually want to see to prove that the testing is
effective).
thanks,
Chris