Looks good.
I'll push it for you.
Best regards,
Vladimir Ivanov
On 4/14/15 2:33 PM, Michael Haupt wrote:
Hi John,
thanks again; I've applied your suggestions, re-tested as before and uploaded
the revision to http://cr.openjdk.java.net/~mhaupt/8076461/webrev.02/.
Best,
Michael
Am 13.04.2015 um 21:38 schrieb John Rose <john.r.r...@oracle.com>:
That's much better; thanks. Glad to hear the verifyC's still works.
The MN_* constants are a private interface between C++ and Java code. Those
are the most important to verify.
You can get rid of these lines; we don't look at vtable indexes any more:
// The JVM uses values of -2 and above for vtable indexes.
// Field values are simple positive offsets.
// Ref: src/share/vm/oops/methodOop.hpp
// This value is negative enough to avoid such numbers,
// but not too negative.
The other constants are publicly defined in various standards docs (except
T_ILLEGAL).
I don't think these constants are used any more, except the MN_* and REF_*
ones. (The REF_* ones are in the JVM standard, so are in some sense
pre-verified.)
I suggest also removing the ACC_*, T_*, and CONSTANT_* names, if you can. We
probably stopped using any of those when we started using ASM.
Thanks!
— John
On Apr 13, 2015, at 4:40 AM, Michael Haupt <michael.ha...@oracle.com> wrote:
Hi John,
thank you very much for your review; keeping the Constants class around for
VM/JDK constant value agreement certainly makes sense. I have undone most of
the removal work and verified in a slowdebug build that MHN.verifyConstants()
works. I've also added a comment on the Constants class to clarify its role a
bit. Local tests and JPRT are still happy with this.
Updated webrev: http://cr.openjdk.java.net/~mhaupt/8076461/webrev.01/
Best,
Michael
Am 07.04.2015 um 23:49 schrieb John Rose <john.r.r...@oracle.com>:
On Apr 7, 2015, at 12:11 PM, Michael Haupt <michael.ha...@oracle.com> wrote:
Dear all,
please review and sponsor this change. Cross-posted to hs-comp and core-lib as
this is at the JVM/libraries boundary. This is a straightforward refactoring
change that removes many constants and unused API from MHNatives, and places
some constants used only in MemberName in that class.
The class MethodHandleNatives.Constants exists to enumerate and cross-check any constants
which the JVM and JDK code need to agree about. Removing a constant from
MethodHandleNatives.Constants (moving to MemberName) may cause failures when
MHN.verifyConstants is run (via "java -esa" on a debug build of Java). If
there are no failures, I wonder what would happen if the JVM and JDK got out of sync. in
their notion of the value of a constant like MN_CALLER_SENSITIVE. It's important that
some part of our release testing detect if MN_CALLER_SENSITIVE (etc.) gets out of sync.
If there is some reason why this testing is no longer needed, I'd like to see
the whole Constants class go away, since that's all it's really good for. But
I don't see that reason yet, and moving the constants somewhere either will
cause a test failure, or *should* cause a test failure.
I'm happy to see the "GC" guys go away. They were artifacts of a quickly
moving 292 implementation that spanned two repositories with unsynchronized change
streams.
— John
RFE: https://bugs.openjdk.java.net/browse/JDK-8076461
Changes: http://cr.openjdk.java.net/~mhaupt/8076461/webrev.00/
Tested with JPRT, HotSpot testset.
Thanks,
Michael
