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

[Paul Sandoz]

Hi Claes,

This looks good.

FWIW VarHandle construction uses native MemberName accessor methods in MethodHandleNatives hence why you did not need to update that code.

Hopefully over time we can further reduce the Unsafe usage in the j.u.concurrent classes.

Paul.


> On 21 Jun 2017, at 05:32, Claes Redestad <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
>>> Hotspot: http://cr.openjdk.java.net/~redestad/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/
> Hotspot: http://cr.openjdk.java.net/~redestad/8182487/hotspot.01/
> 
> Thanks!
> 
> /Claes