RFR: 5043030 (reflect) unnecessary object creation in reflection

[David Holmes]

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.

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
>