Hi Serguei, > On Jun 22, 2016, at 1:17 PM, serguei.spit...@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.spit...@oracle.com>serguei.spit...@oracle.com >>> <mailto:serguei.spit...@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.spit...@oracle.com >>> <mailto:serguei.spit...@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 >>> >> >
