RFR: 8168850: Mark module entries that have been specified by --patch-module

[Lois Foltan]

On 12/5/2016 12:34 AM, Ioi Lam wrote:
>
>
> On 12/1/16 5:44 PM, David Holmes wrote:
>> Hi Rachel,
>>
>> On 2/12/2016 7:18 AM, Rachel Protacio wrote:
>>> Hi,
>>>
>>> Please review this fix, which adds a boolean to ModuleEntry to specify
>>> whether the module has been patched using the command line
>>> --patch-module. This is needed for CDS to know whether to load a class
>>> from the shared archive. Their tests for those changes will 
>>> encompass my
>>> change here. I verified that it worked properly through the logging
>>> messages, which can be triggered with
>>>     -Xlog:modules+patch=trace
>>> Passes JPRT.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8168850
>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8168850.00/
>>
>> Looks okay.
>>
>> My only query is with the change in the initialization order for 
>> create_javabase. Does JPRT testing sufficiently exercise this?
>>
>
> The change in the initialization seems OK to me. 
> setup_patch_mod_entries() doesn't seem to rely on the result of 
> create_javabase(), so it should be OK to move create_javabase() after 
> setup_patch_mod_entries().

I agree. I suggested moving the creation of the java.base ModuleEntry 
after setup_patch_mod_entries in order to eliminate special code to 
handle the setting of java.base's ModuleEntry::_is_patched field.  By 
moving the call after setup_patch_mode_entries, all modules can have 
their _is_patched field set via ModuleEntryTable::new_entry().  And as 
Ioi points test cases already exist that test patching java.base.  It 
might be good if Rachel enhanced the comment at line #1789 in 
ClassLoader::classLoader_init2() to indicate that the creation of the 
java.base ModuleEntry must occur after setup_patch_mod_entries in order 
to successfully determine if java.base has been patched.

Thanks,
Lois

>
> The JPRT tests usually run without --patch-modules, but also uses 
> --patch-modules in some test cases:
>
> $grep Patch linux_x64_3.8-fastdebug-c2-hotspot_fast_runtime.log
> TEST: runtime/modules/PatchModule/PatchModuleCDS.java
> TEST: runtime/modules/PatchModule/PatchModuleDupJavaBase.java
> TEST: runtime/modules/PatchModule/PatchModule2Dirs.java
> TEST: runtime/modules/PatchModule/PatchModuleClassList.java
> TEST: runtime/modules/PatchModule/PatchModuleDupModule.java
> TEST: runtime/modules/PatchModule/PatchModuleJavaBase.java
> TEST: runtime/modules/PatchModule/PatchModuleTest.java
> TEST: runtime/modules/PatchModule/PatchModuleTestJar.java
> TEST: runtime/modules/PatchModule/PatchModuleTestJarDir.java
> TEST: runtime/modules/PatchModule/PatchModuleTraceCL.java
> TEST: runtime/modules/Visibility/PatchModuleVisibility.java
>
> So I think the coverage is pretty good.
>
> Thanks
> - Ioi
>
>> Thanks,
>> David
>>
>>> Thank you!
>>> Rachel
>