RFR(L) 8136930: Simplify use of module-system options by custom launchers

Lois Foltan lois.foltan at oracle.com
Tue Aug 2 00:40:48 UTC 2016


On 7/17/2016 7:05 PM, harold seigel wrote:
> Hi,
>
> Please review these Hotspot VM only changes to process the seven 
> module-specific options that have been renamed to have gnu-like 
> names.  JDK changes for this bug will be reviewed separately.
>
> Descriptions of these options are here 
> <http://openjdk.java.net/jeps/293>.  For these six options, 
> --module-path, --upgrade-module-path, --add-modules, --limit-modules, 
> --add-reads, and --add-exports, the JVM just sets a system property.  
> For the --patch-module option, the JVM sets a system property and then 
> processes the option in the same way as when it was named -Xpatch.
>
> Additionally, the JVM now checks properties specified on the command 
> line.  If a property matches one of the properties used by one of the 
> above options then the JVM ignores the property.  This forces users to 
> use the explicit option when wanting to do things like add a module or 
> a package export.
>
> The RFR contains two new tests.  Also, many existing tests were 
> changed to use the new option names.
>
> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8136930
>
> Webrev: http://cr.openjdk.java.net/~hseigel/bug_8136930.hs/

Hi Harold,

Overall looks good.  A couple of comments:

src/share/vm/prims/jvmtiEnv.cpp
- line #3428 - The if statement is incorrect.  There are internal 
properties, like jdk.boot.class.path.append, whose value if non-null 
should be returned.

src/share/vm/runtime/arguments.cpp
- Arguments::append_to_addmods_property was added before the VM starting 
to process --add-modules.  So with this fix, it seems like it could be 
simply changed to:

    bool Arguments::append_to_addmods_property(const char* module_name) {
       PropertyList_unique_add(&_system_properties,
    Arguments::get_property("jdk.module.addmods"),
                                                    module_name,
    AppendProperty, UnwriteableProperty, InternalProperty);
    }

Please consider making this change since currently it contains a lot of 
duplicated code that is now unnecessary.

- line #3171, should the comment be "--add-modules=java.sql" instead of 
"--add-modules java.sql"?

Thanks,
Lois













>
> The changes were tested with the JCK lang and VM tests, the JTreg 
> hotspot tests, and the RBT hotspot nightlies.
>
> Thanks, Harold



More information about the hotspot-runtime-dev mailing list