Please review: VM side of Jigsaw classpath mode support

[Karen Kinnear]

Mandy,

Thank you for the review and for describing the internal 
system properties as used so far.
On Aug 24, 2012, at 4:59 PM, Mandy Chung wrote:

> On 8/23/2012 1:14 PM, Karen Kinnear wrote:
>> The VM side of this webrev is at:
>>     http://cr.openjdk.java.net/~acorn/vm.classpath/webrev/
> Looks good, Karen.  I should take this opportunity to describe
> the new properties VM sets for JDK to use but they are removed
> from System properties at startup time
> 
> sun.boot.class.prepend.path - paths set by -Xbootclasspath/p
> sun.boot.class.append.path - paths set by -Xbootclasspath/a and
>  added by JVMTI or java.lang.instrument.
>  The above 2 are used for searching system resources.
>  It used to search system resources from "sun.boot.class.path"
>  but in a modular JDK, we need to separate the prepend and
>  append paths.  We take this simple approach to pass these 2
>  properties to the JDK properties since the VM already has
>  the separated search paths.
> 
> sun.boot.module.classpath - Class.getPackage() calls JVM_GetSystemPackage
>  to obtain the source where a system class is loaded (it may
>  be a JAR file or a directory).  If a system class is loaded
>  from a module image in classpath mode, it can be loaded from
>  any module for the runtime.  The current VM implementation
>  only supports an entry to the bootclasspath be appended.
>  In addition, there is no manifest for modules anyway. All
>  we need is to tell the JDK that this system class is loaded
>  from modules; then it will construct a Package instance
>  based on the system properties.  So sun.boot.module.classpath
>  serves as a unique tag for JDK to compare with the returned
>  value from JVM_GetSystemPackage.
> 
> I have a few minor comments and you can be addressed in
> future revisions.  There will be another update for modulepath
> support next.
> 
> In os.cpp, does it need to reset UseModuleNativeLibs if there
> is no base module?

>  You already check for the flag and
> has_module_image():
>  if (UseModuleNativeLibs&&  Arguments::has_module_image())
> 
> os.cpp L1140-1142 - I thought this is no longer needed and can be
> removed.
Due to the incremental bootstrapping steps, I would like to revisit
the exact order in which each of these decisions is made and try
to make this a bit cleaner.  I would like to do that in the context
of the upcoming modulepath changes since that may modify some of this
approach. So I will note your suggestions and revisit them.

> 
> classLoader.hpp looks like "_boot_module_source" is not used.
Good catch. Thank you.
> BTW, should Arguments::boot_module_index() belong to classLoader.hpp?
I am anticipating that that is temporary and may change due to
modulepath support to support a list of modules. I expect that will
need to be shared between the launcher, jvm and jdk in some form.

> 
> It's an open issue what "sun.boot.class.path" should be.
> I believe your current patch includes the legacy rt.jar and other
> paths in sun.boot.class.path because you want the VM to be able
> to run on a legacy image (e.g. JDK8).  Perhaps you can detect from
> JDK_Version if it's a jigsaw build, not to include the legacy paths
> in sun.boot.class.path.  I have been considering adding some interface
> to indicate it's a modular image (since the JDK release is still 8
> in our build).  I can add a bit in the jdk_version_info in our next
> revision.
Yes, the jvm needs to be able to run with rt.jar or with the module image
for now. We can discuss possible approaches.
> 
> Thanks
> Mandy
thanks,
Karen
> 
>