RFR (S): 8076461: JSR292: remove unused native and constants

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Tue Apr 14 11:47:51 UTC 2015


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.rose at 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.haupt at 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.rose at oracle.com>:
>>>>
>>>> On Apr 7, 2015, at 12:11 PM, Michael Haupt <michael.haupt at 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
>
>


More information about the hotspot-compiler-dev mailing list