Hi David,
The refactoring in jvmtiRedefineClasses.cpp looks great!
The rest looks good too.
Thanks,
Serguei
On 5/21/18 21:41, David Holmes wrote:
Here are the minor updates to the serviceability code based on all the
code reviews so far:
Incremental webrev:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.serviceability.v2-incr/
Full webrev:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.serviceability.v2/
Changes:
src/java.instrument/share/classes/java/lang/instrument/Instrumentation.java
Fixed typo: retransformation -> redefinition
Fixed typo: maybe be -> may be
---
src/hotspot/share/prims/jvmtiRedefineClasses.cpp
- removed commented out old code
- refactored nest attribute checks into separate function
- removed unneeded RedefineVerifyMark
---
test/jdk/com/sun/jdi/RedefineNestmateAttr/TestNestmateAttr.java
Fixed double-spaces in comments
---
Thanks,
David
-----
On 15/05/2018 10:52 AM, David Holmes wrote:
This review is being spread across four groups: langtools, core-libs,
hotspot and serviceability. This is the specific review thread for
serviceability - webrev:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.serviceability.v1/
See below for full details - including annotated full webrev guiding
the review.
The intent is to have JEP-181 targeted and integrated by the end of
this month.
Thanks,
David
-----
The nestmates project (JEP-181) introduces new classfile attributes
to identify classes and interfaces in the same nest, so that the VM
can perform access control based on those attributes and so allow
direct private access between nestmates without requiring javac to
generate synthetic accessor methods. These access control changes
also extend to core reflection and the MethodHandle.Lookup contexts.
Direct private calls between nestmates requires a more general
calling context than is permitted by invokespecial, and so the JVMS
is updated to allow, and javac updated to use, invokevirtual and
invokeinterface for private class and interface method calls
respectively. These changed semantics also extend to MethodHandle
findXXX operations.
At this time we are only concerned with static nest definitions,
which map to a top-level class/interface as the nest-host and all its
nested types as nest-members.
Please see the JEP for further details.
JEP: https://bugs.openjdk.java.net/browse/JDK-8046171
Bug: https://bugs.openjdk.java.net/browse/JDK-8010319
CSR: https://bugs.openjdk.java.net/browse/JDK-8197445
All of the specification changes have been previously been worked out
by the Valhalla Project Expert Group, and the implementation reviewed
by the various contributors and discussed on the valhalla-dev mailing
list.
Acknowledgments and contributions: Alex Buckley, Maurizio Cimadamore,
Mandy Chung, Tobias Hartmann, Vladimir Ivanov, Karen Kinnear,
Vladimir Kozlov, John Rose, Dan Smith, Serguei Spitsyn, Kumar Srinivasan
Master webrev of all changes:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.full.v1/
Annotated master webrev index:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/jep181-webrev.html
Performance: this is expected to be performance neutral in a general
sense. Benchmarking and performance runs are about to start.
Testing Discussion:
------------------
The testing for nestmates can be broken into four main groups:
- New tests specifically related to nestmates and currently in the
runtime/Nestmates directory
- New tests to complement existing tests by adding in testcases not
previously expressible.
- For example java/lang/invoke/SpecialInterfaceCall.java tests
use of invokespecial for private interface methods and performing
receiver typechecks, so we add
java/lang/invoke/PrivateInterfaceCall.java to do similar tests for
invokeinterface.
- New JVM TI tests to verify the spec changes related to nest
attributes.
- Existing tests significantly affected by the nestmates changes,
primarily:
- runtime/SelectionResolution
In most cases the nestmate changes makes certain invocations that
were illegal, legal (e.g. not requiring invokespecial to invoke
private interface methods; allowing access to private members via
reflection/Methodhandles that were previously not allowed).
- Existing tests incidentally affected by the nestmate changes
This includes tests of things utilising class
redefinition/retransformation to alter nested types but which
unintentionally alter nest relationships (which is not permitted).
There are still a number of tests problem-listed with issues filed
against them to have them adapted to work with nestmates. Some of
these are intended to be addressed in the short-term, while some
(such as the runtime/SelectionResolution test changes) may not
eventuate.
- https://bugs.openjdk.java.net/browse/JDK-8203033
- https://bugs.openjdk.java.net/browse/JDK-8199450
- https://bugs.openjdk.java.net/browse/JDK-8196855
- https://bugs.openjdk.java.net/browse/JDK-8194857
- https://bugs.openjdk.java.net/browse/JDK-8187655
There is also further test work still to be completed (the JNI and
JDI invocation tests):
- https://bugs.openjdk.java.net/browse/JDK-8191117
which will continue in parallel with the main RFR.
Pre-integration Testing:
- General:
- Mach5: hs/jdk tier1,2
- Mach5: hs-nightly (tiers 1 -3)
- Targetted
- nashorn (for asm changes)
- hotspot: runtime/*
serviceability/*
compiler/*
vmTestbase/*
- jdk: java/lang/invoke/*
java/lang/reflect/*
java/lang/instrument/*
java/lang/Class/*
java/lang/management/*
- langtools: tools/javac
tools/javap