[10] RFR: 8182487: Add Unsafe.objectFieldOffset(Class, String)

[Claes Redestad]

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.redestad at oracle.com <mailto:claes.redestad at 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
>
>