preliminary RFR: 8049365 - Update JDI and JDWP for modules
Alan.Bateman at oracle.com
Thu Dec 10 12:04:34 UTC 2015
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 functionality.
> 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,
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 1.9.
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