RFR: 8263595: Remove oop type punning in JavaCallArguments

[Stefan Karlsson]

On Mon, 15 Mar 2021 16:44:05 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> JavaCallArguments has this code and comment:
>> 
>> // Helper for push_oop and the like. The value argument is a
>> // "handle" that refers to an oop. We record the address of the
>> // handle rather than the designated oop. The handle is later
>> // resolved to the oop by parameters(). This delays the exposure of
>> // naked oops until it is GC-safe.
>> template<typename T>
>> inline int push_oop_impl(T handle, int size) {
>>   // JNITypes::put_obj expects an oop value, so we play fast and
>>   // loose with the type system. The cast from handle type to oop
>>   // *must* use a C-style cast. In a product build it performs a
>>   // reinterpret_cast. In a debug build (more accurately, in a
>>   // CHECK_UNHANDLED_OOPS build) it performs a static_cast, invoking
>>   // the debug-only oop class's conversion from void* constructor.
>>   JNITypes::put_obj((oop)handle, _value, size); // Updates size.
>>   return size; // Return the updated size.
>> }
>> The type T is either an oop* or jobject (JNI handle). This puts something that isn't an oop inside an oop.
>> 
>> I propose that we don't do this. Instead we could pass the handle (address containing the oop), and then in put_obj convert that address to an intptr_t, which matches well with the `to` argument of those functions.
>> 
>> I've been running this (and some other changes) with ZGC on Linux x64 through tier1-tier7.
>
> src/hotspot/share/runtime/javaCalls.hpp line 110:
> 
>> 108:   // handle rather than the designated oop.  The handle is later
>> 109:   // resolved to the oop by parameters().  This delays the exposure of
>> 110:   // naked oops until it is GC-safe.
> 
> I thought this was the reason we had to have this ugliness.  Once you push an oop to the argument stack, you could have a GC and this was a naked oop.  By having the Handle pointer or jobject pointer, you'd get a pointer to the oop for GC to process.  Maybe this change does the same thing though, only a lot nicer. Yes, it looks like it does.  Now to have Kim point out why it's wrong :(  I hope not because I like this change.

The patch does the same thing as previously, but it skips casting oop*/jobject to oop.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3014