I'm happy with these changes, even though it will make jsr166 maintainers
lives more difficult due to jdk9/jdk10 divergence - we will adapt. We
would like to see this backported to jdk9, but too late for that.
---
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) {
On Wed, Jun 21, 2017 at 5:32 AM, Claes Redestad <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
>>> 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
>
