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

[Jiangli Zhou]

Hi Serguei,

> On Jun 22, 2016, at 1:17 PM, serguei.spitsyn at oracle.com wrote:
> 
> Hi Jiangli,
> 
> Fixed both places - nice catch!
> Would you like I list you as a reviewer?

No worry, if you already committed/pushed your changes.

Thanks,
Jiangli

> 
> 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,  <mailto:serguei.spitsyn at oracle.com>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/ <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 <mailto:serguei.spitsyn at oracle.com> wrote:
>>>> 
>>>> Please, review the Jigsaw fix for the enhancement:
>>>> https://bugs.openjdk.java.net/browse/JDK-8159145 <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/ <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/ <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
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20160622/f9226087/attachment.html>