[9] RFR (M) 8054386: Allow Java debugging when CDS is enabled

David Holmes david.holmes at oracle.com
Fri Jun 5 05:50:13 UTC 2015


On 5/06/2015 9:32 AM, Chris Plummer wrote:
> Hi David,
>
> Here's an updated webrev:
>
> http://cr.openjdk.java.net/~cjplummer/8054386/webrev.04/

Thanks - should have said updated webrev not necessary :)

Looks good.

David

> thanks,
>
> Chris
>
> On 6/3/15 11:29 PM, David Holmes wrote:
>> Hi Chris,
>>
>> Hotspot change is good.
>>
>> Only a couple of style nits with the tests (where are our Java style
>> guidelines ???). Taking CDSJDITest.java as an example:
>>
>> If you are okay with this line length:
>>
>>  115     public static OutputAnalyzer executeAndLog(ProcessBuilder pb,
>> String logName) throws Exception {
>>
>> then you can remove a number of line breaks in the headers of other
>> functions. (I don't follow the 70-80 char line length  dogma ;) )
>>
>> If you do break a header of a function the { still stays on the same
>> line as the last header component ie:
>>
>>      private static void addToClassList(PrintStream ps, String classes[])
>>          throws IOException {
>>
>> not:
>>
>>  139     private static void addToClassList(PrintStream ps, String
>> classes[])
>>  140         throws IOException
>>  141     {
>>
>> Cheers,
>> David
>>
>> On 2/06/2015 5:36 PM, Chris Plummer wrote:
>>> [Adding core-libs-dev at 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
>>>>
>>>
>


More information about the hotspot-runtime-dev mailing list