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 -- <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 Oracle Java Platform Group | HotSpot Compiler Team Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
