RFR: 5043030 (reflect) unnecessary object creation in reflection

[David Holmes]

Hi Roger,

On 3/06/2014 12:15 AM, roger riggs wrote:
> Hi David, et.al.
>
> I would let the compiler do auto-boxing where necessary.  (Assuming
> object identity is not necessary).

I don't see where the compiler comes into this one. ???

David
-----

> If type disambiguation is necessary then use a cast to the target type and
> let the compiler do the rest. It keeps the source code simple and readable.
>
> But I don't think it is worth a proactive pervasive change.
> The gain is overshadowed by the overhead of the reviews.
>
> $.02, Roger
>
>
>
> On 6/1/2014 11:07 PM, David Holmes wrote:
>> Hi Andrej,
>>
>> Sorry for the delay getting back to you.
>>
>> On 29/05/2014 10:24 PM, Andrej Golovnin wrote:
>>> Hi David,
>>>
>>>>>
>>>>> The valueOf calls may also allocate a new object so you can't just
>>>>> delete the JvmtiExport::post_vm_object_alloc call. Unfortunately you
>>>>> can't tell whether a new object was allocated or not. It is only
>>>>> for the
>>>>> smaller primitive types that any kind of Object caching is mandated.
>>>>
>>>> It is only for the smaller values (-128 to +127) of the integer
>>>> primitives types (plus boolean) that caching is mandated.
>>>> Float.valueOf and Double.valueOf always create objects.
>>>>
>>>
>>> You are right, that #valueOf call may allocate an object.
>>> But as far as I understand currently the
>>> JvmtiExport::post_vm_object_alloc call
>>> is only needed, because today the native code itself allocates an object
>>> by calling java_lang_boxing_object::create(type, value, CHECK_NULL);.
>>
>> Right, sorry - I was misunderstanding the purpose of the
>> post_vm_object_alloc call:
>>
>> http://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#VMObjectAlloc
>>
>>
>> So from the perspective that you are diverting this back to Java code
>> the hotspot changes look okay to me.
>>
>> The more general question, for the core-libs folk, is whether changing
>> everything to use valueOf is overkill (given the limits of the
>> required caching mechanisms) or good to do from a consistency
>> perspective. I'm slightly on the overkill side of things but not
>> enough to reject things.
>>
>> On the performance/benefit side, if I read things correctly you only
>> see the 9GB of Boolean objects because you disable
>> reflection-inflation - is that right? In that case, as Joel states,
>> the gains are not really general, but on the other hand I don't see
>> anything wrong with trying to improve the general efficiency here even
>> if the greatest benefit comes from a "non-mainstream" usecase.
>>
>> David
>> -----
>>
>>> My code changes this behavior and delegates object allocation back to
>>> Java
>>> by calling
>>>
>>>    JavaCalls::call_static(&boxed_value,
>>>                           klass_handle,
>>>                           vmSymbols::valueOf_name(),
>>>                           valueOf_signature,
>>>                           &args,
>>>                           THREAD);
>>>
>>> But maybe I misunderstood the implementation of JavaCalls.
>>>
>>> Best regards,
>>> Andrej Golovnin
>>>
>