preliminary RFR: 8049365 - Update JDI and JDWP for modules
harold.seigel at oracle.com
Thu Dec 10 19:57:19 UTC 2015
Looks good. Just a few comments.
1. Should jni_GetAllModules() contain the DTrace macros that other JNI
2. Can you add a similar GetAllModules() function to jniCheck.cpp ?
On 12/10/2015 7:04 AM, Alan Bateman wrote:
> On 10/12/2015 08:03, serguei.spitsyn at oracle.com wrote:
>> Jdk webrev:
>> Hotspot webrev:
>> It is expected that the JDI and JDWP update for modules will have
>> several iterations.
>> This is the initial one, and it introduces a very minimal
>> The main purpose of this preliminary review is to make sure the JDI
>> and JDWP update
>> for modules goes in a right direction and has nothing obviously wrong.
> I went through the changes to the JDWP spec + additions to jdwpgen in
> this iteration and it looks quite good. A few comments:
> 1. I don't know if JEP 223 considered the JDWP version but I will
> assume this will move to 9 instead of 1.9.
> 2. In the Module command then the description would be clearer as "...
> that this reference type ...".
> 3. In the Module command set then we'll need to decide the reply to
> the Name command for the case that the module is an unnamed module.
> There is also an open issue for the runtime API too.
> 4. Also in the Module command set then the CanRead targetModule should
> be "source" (not target) to get consistency with the runtime API.
> As you mentioned in your mail, I think the GetAllModules will need to
> move to JVM TI so that it's consistent with GetAllClasses,
> GetAllThreads, ...
> I also went through the JDI changes. The type hierarchy in JDI is
> quite deep and is has a convention for naming that we need to follow
> too. Looking at it now then it looks like it should be ModuleReference
> extends ObjectReference. That would give us consistency with other
> mirrors that are object references (ThreadReference and
> ClassLoaderReference for example). We don't rev JDI often enough to
> keep the hierarchy and naming convention in our heads so good to have
> more eyes on this.
> ReferenceType::module looks right, we'll just need to iterate on the
> VirtualMachine::allModules looks right. Note that the TRACE_* fields
> are public and part of the JDI API so we'll need to decide whether
> TRACE_MODULES is important or not.
> In ReferenceTypeImpl then I assume isModuleCached is not needed. Maybe
> you have this to the JDWP command when connected to a target VM that
> is JDK 8 or older?
> VirtualMachineImpl - this might be the time to change to 9 rather than
> ModuleImpl - I assume ref can be final.
> Have you thought about SA yet? I can't recall if it is compiled with
> the boot JDK or will be compiled against the newly built jdk.jdi
> module. If the later then I assume that SA will need updates. If the
> former then I assume we will have issues with boot cycle builds.
> Tests. I see you have a basic test but I assume we will need to find
> time to develop a lot more tests for this update.
More information about the jigsaw-dev