Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

[serguei.spitsyn at oracle.com]

Hi Jiangli,

Fixed both places - nice catch!
Would you like I list you as a reviewer?

Thanks,
Serguei


On 6/22/16 10:29, Jiangli Zhou wrote:
> Hi Serguei,
>
> The comment in the following assert is outdated. There is no 
> get_module_from_pkg. Should it be changed to get_module_by_package_name()?
>
> 804 assert(ModuleEntryTable::javabase_defined(),
> 805 "Attempt to call get_module_from_pkg before java.base is defined");
>
> It’s not part of your change, I just happen to notice it. In the same 
> get_module_by_package_name(), both the ‘if’ and ‘else’ cases have 
> return statement. The ‘return NULL’ at line 825 seems will never 
> reached and not needed.
>
> Thanks,
> Jiangli
>
>> On Jun 22, 2016, at 4:07 AM, serguei.spitsyn at oracle.com 
>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>
>> Here are new hotspot webrev where I've fixed the comments from Alan 
>> and Christian.
>>
>> Hotspot:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.2 
>> <http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.2>
>>
>>
>> The Jdk webrev is the same:
>> http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/
>>
>>
>> Thanks,
>> Serguei
>>
>>
>> On 6/21/16 02:54, serguei.spitsyn at oracle.com wrote:
>>>
>>> Please, review the Jigsaw fix for the enhancement:
>>> https://bugs.openjdk.java.net/browse/JDK-8159145
>>>
>>>
>>> The Hotspot webrev:
>>> http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/
>>>
>>> The Jdk webrev:
>>> http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/
>>>
>>>
>>> Summary:
>>>
>>> JVM TI agents that instrument code in named modules need the Module 
>>> at class load time.
>>>
>>>  One way to do this is by introducing a new ClassFileLoadHook that 
>>> takes an additional parameter but this approach is disruptive.
>>>  The alternative option is a JVM TI function that maps a classloader 
>>> + package name to a module.
>>>  We were initially not confident with this approach so we introduced 
>>> it as JVM function JVM_GetModuleByPackageName.
>>>  Based on experience to date then this approach seems okay and so 
>>> this function needs to be promoted to a JVMTI function.
>>>
>>>  This fix is to introduce new JVMTI function GetModuleByPackageName.
>>>  It includes new jtreg test with native JVMTI agent.
>>>
>>>  A CCC is fast-tracked and getting an approval is in progress.
>>>
>>> Testing:
>>>   Run newly developed jtreg test.
>>>
>>> Thanks,
>>> Serguei
>>
>