On 06/08/2017 01:37, mandy chung wrote:


It is good to see more optimization be done at link time that improves the startup.

jdk/internal/loader/ClassLoaders.java
79 if (cp.length() == 0) cp = null;
Our launcher and hotspot VM always set "java.class.path" system property and so it'll be non-null. It might help the readers to check if (cp != null && cp.length() == 0).
Fair point, there is an assumption that the VM is created with either an initial module or a class path and HotSpot masks that to some extent. So yes, clearer if we use the above.


jdk/internal/module/SystemModulesMap.java
128 Constructor<?> ctor = Class.forName(cn).getConstructor();
Class.forName(Module, String) will only search the specified module. No performance difference since the class exists in java.base but it may make it clear to the readers.
The loaded classes haven't been patched to be members of java.base at this point in the startup so there isn't a Module object to use the 2-arg forName. But yes, a comment might help here.


jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
This generates one SystemModules for each module that has a main class. A user may create a launcher at link time but specifies a main class which may be different to the one in the module descriptor. A possible future enhancement is to generate a SystemModules for the main class if the launcher is created by jlink.
Yes, I've been thinking about this too and I think it will require a clean way for jlink to expose the details of launchers to plugins.

Otherwise, this patch looks good.
Thanks for spending time on this.

-Alan

Reply via email to