RFR (S) CR 8016236: Class.getGenericInterfaces performance improvement
Peter Levart
peter.levart at gmail.com
Mon Jun 17 10:20:10 UTC 2013
On 06/17/2013 10:06 AM, Aleksey Shipilev wrote:
> On 06/16/2013 11:44 PM, Alan Bateman wrote:
>> On 13/06/2013 12:47, Aleksey Shipilev wrote:
>>> Friendly reminder for the reviewers.
>>>
>>> On 06/10/2013 07:53 PM, Aleksey Shipilev wrote:
>>>> This is the follow-up on the issue Doug identified:
>>>>
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017798.html
>>>>
>>>>
>>>> I had reworked the patch, webrev is here:
>>>> http://cr.openjdk.java.net/~shade/8016236/webrev.01/
>>> The current webrev is here:
>>> http://cr.openjdk.java.net/~shade/8016236/webrev.02/
>>>
>> I'm just catching up on this thread, webrev.02 looks very good to me. I
>> can sponsor it.
> Thanks! I'll appreciate this.
>
>> One comment on ClassRepositoryHolder is that it's "yet another class"
>> (we have a proliferation of holder classes since we started using this
>> idiom). It makes me wonder if it might be better to move the constant to
>> ClassRepository.
> Ok, makes sense. I moved it to ClassRepository itself. Given we have the
> ClassRepository field in Class, the static initialization will be
> performed on the first class load. I have no problems having it public,
> since it is protected enough from the external modifications.
Sorry, Aleksey, just a nit: In getInterfaces(), the method could be
written using a single volatile read from ReflectionData.interfaces
field instead of two (by introducing local variable).
Regards, Peter
>
> The new webrev is here:
> http://cr.openjdk.java.net/~shade/8016236/webrev.03/
>
> Testing:
> - Linux/x86_64/release build OK
> - Linux/x86_64/release passes java/lang/reflect jtreg
> - microbenchmark scores are still intact
>
> -Aleksey.
More information about the core-libs-dev
mailing list