RFR: 5043030 (reflect) unnecessary object creation in reflection

[Joel Borggrén-Franck]

Hello,

On 29 maj 2014, at 10:23, Andrej Golovnin <andrej.golovnin at gmail.com> wrote:

> Hi Joel,
> 
>> We need you send in the patches via the mailing list. Please reply to your mail with the diffs attached directly (not zipped for example).
> 
> OK, will do that. But I always thought a webrev is
> the preferred and the best way to submit a patch.

You are right that webrevs are preferred for reviewing, but we also need the contribution submitted through openjdk infrastructure. BTW it looks like your attachments were stripped.

>> I’ll might have time to look at this next week, but it surprises me you need to patch hotspot. Allocations before we have inflated shouldn’t be a problem.
>> Also do you have any data showing that this actually makes a difference?
> 
> 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.

> The returned value must be wrapped. Currently this is
> done by the following call
> 
> java_lang_boxing_object::create(type, value, CHECK_NULL);
> 
> in reflection.cpp. This method allocates a new object.
> 
> As I wrote in my previous mail, the JDK patch contains
> two regression tests for jtreg to verify my changes.
> If you run this tests using current JDK9, than the tests will fail.
> After applying my patches the tests executed without any problem.

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, you must show "real world” benefit. Ideally you should be able to show significant reduction in allocation in some application.

cheers
/Joel