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

[Rachel Protacio]

Lois, Jiangli, and David, thanks for your reviews. And thank you Ioi for 
your test knowledge! Since it seems like there are no other concerns, 
I'll update that comment as Lois suggested and commit.

Rachel


On 12/5/2016 7:52 AM, Lois Foltan wrote:
>
> 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
>>
>