RFR: 5043030 (reflect) unnecessary object creation in reflection

[David Holmes]

Correction ...

On 29/05/2014 9:06 PM, David Holmes wrote:
> Hi Andrej,
>
> The hotspot changes need to be reviewed by hotspot developers so I've
> cc'd the runtime team.
>
> On 29/05/2014 8:45 PM, Andrej Golovnin wrote:
>> Hi Joel,
>>
>>>> When you have for example following method:
>>>>
>>>> public int method() {
>>>> return 0;
>>>> }
>>>>
>>>> And you invoke this method over the reflection API,
>>>> then the first N invocations are done via the native code.
>>>
>>> Yes, this is before inflation. Inflation happens after 15 calls IIRC,
>>> which is why I don’t think it matters from a performance standpoint.
>>
>> I would say, It depends on how do you define "matters". I personally
>> don't care
>> about the native part in this case, as I always set
>> "sun.reflect.noInflation=true".
>> But I think the changes to Hotspot are just needed for the correctness
>> of the fix.
>
> 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.

David

> David
> -----
>
>>>
>>> Your tests show that the valueOf caches are used which is good.
>>> However the byte code spinning is a core piece of reflection that is
>>> currently in production in countless places. A change in this area
>>> should not only be obviously correct and thouroghly tested
>>
>> That's why we do the code review. ;-)
>>
>>> , you must show "real world” benefit. Ideally you should be able to
>>> show significant reduction in allocation in some application.
>>
>> I'm working on a product which has ca. 3 million lines of code and make
>> direct and indirect use of Reflection API. And in one of our use cases
>> I see, that JVM allocates ca. 9GB of Boolean objects. All of them are
>> allocated through the usage of Reflection API. Even that most of this
>> objects are allocated in TLABs and are removed by GC when the use
>> case is finished, I would say we have allocated 9GB of Boolean objects
>> to much. Or do you see it differently?
>>
>> Let me know, if you need more data or if I should write some test.
>>
>> If don't want accept the patch, then it's OK. But in this case you
>> should close the issue 5043030 and explain why it won't be fixed.
>>
>> Best regards,
>> Andrej Golovnin
>>