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.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 > > > 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.spit...@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 >
