On 2017-06-21 17:13, Martin Buchholz wrote:
I'm happy with these changes, even though it will make jsr166 maintainers lives more difficult due to jdk9/jdk10 divergence - we will adapt.

Glad to hear that!

We would like to see this backported to jdk9, but too late for that.

Yes, I'm afraid so.


---

I would return jlong for consistency. Others have decided long ago that jlong is the type to use here.
+static jint find_field_offset(jclass clazz, jstring name, TRAPS) {

Right, the inconsistency between the existing find_field_offset method returning jint and the JNI method returning jlong is and was surprising. I'll fix.

/Claes


On Wed, Jun 21, 2017 at 5:32 AM, Claes Redestad <claes.redes...@oracle.com <mailto:claes.redes...@oracle.com>> wrote:

    Hi Mikael,

    On 06/21/2017 09:49 AM, Mikael Gerdin wrote:

        Hi Claes,

        This looks like a good change but I do have some comments on
        exception management details.

        On 2017-06-20 15:14, Claes Redestad wrote:

            Hi,

            as a startup optimization, we can avoid a number of
            reflective operations on
            variouscore classes by adding a specialized
            objectFieldOffset method taking
            Class and String rather than Field:

            Webrev: https://bugs.openjdk.java.net/browse/JDK-8182487
            <https://bugs.openjdk.java.net/browse/JDK-8182487>
            Hotspot:
            http://cr.openjdk.java.net/~redestad/8182487/hotspot.00
            <http://cr.openjdk.java.net/%7Eredestad/8182487/hotspot.00>


        The UNSAFE_ENTRY macro expands to among other things a
        transition into _thread_in_vm (normal JNI code runs as
        _thread_in_native).
        When running "in VM" we are (fairly) free to do things such as
        resolving JNI handles and looking at the fields of classes so
        that is clearly correct for find_field_offset.
        For code running in VM we have to avoid invoking functions in
        the public JNI api since those entry points perform
        transitions from native to VM, causing assertions to fire and
        other potentially nasty stuff happening.

        Instead of reusing the throw_new helper function you should do
        something similar to the other find_field_offset function and do

        if (offset < 0) {
          THROW_0(vmSymbols::java_lang_InternalError());
        }
        return field_offset_from_byte_offset(offset);


    makes sense.

    I've also cleaned up and removed static blocks as suggested by
    Brent Christian, and even though I reverted the throw_new method
    back to its original place I think it's reasonable to clean it up as
    suggested by Christian Thalinger:

    JDK: http://cr.openjdk.java.net/~redestad/8182487/jdk.01/
    <http://cr.openjdk.java.net/%7Eredestad/8182487/jdk.01/>
    Hotspot: http://cr.openjdk.java.net/~redestad/8182487/hotspot.01/
    <http://cr.openjdk.java.net/%7Eredestad/8182487/hotspot.01/>

    Thanks!

    /Claes



Reply via email to