On 6/22/16 13:54, Jiangli Zhou wrote:
Hi Serguei,
On Jun 22, 2016, at 1:17 PM, serguei.spit...@oracle.com
<mailto: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.
I've not committed/pushed yet.
Thanks,
Serguei
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, 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/
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