bottleneck by java.lang.Class.getAnnotations() - rebased patch

[David Holmes]

Hi Peter,

Sorry for the delay on this.

Generally your VolatileData and my ReflectionHelper are doing a similar 
job. But I agree with your reasoning that all of the cached 
SoftReferences are likely to be cleared at once, and so a SoftReference 
to a helper object with direct references, is more effective than a 
direct reference to a helper object with SoftReferences. My initial 
stance with this was very conservative as the more change that is 
introduced the more uncertainty there is about the impact.

I say the above primarily because I think, if I am to proceed with this, 
I will need to separate out the general reflection caching changes from 
the annotation changes. There are a number of reasons for this:

First, I'm not at all familiar with the implementation of annotations at 
the VM or Java level, and the recent changes in this area just 
exacerbate my ignorance of the mechanics. So I don't feel qualified to 
evaluate that aspect.

Second, the bulk of the reflection caching code is simplified by the 
fact that due to current constraints on class redefinition the caching 
is effectively idempotent for fields/methods/constructors. But that is 
not the case for annotations.

Finally, the use of synchronization with the annotations method is 
perplexing me. I sent Joe a private email on this but I may as well 
raise it here - and I think you have alluded to this in your earlier 
emails as well: initAnnotationsIfNecessary() is a synchronized instance 
method but I can not find any other code in the VM that synchronizes on 
the Class object's monitor. So if this synchronization is trying to 
establish consistency in the face of class redefinition, I do not see 
where class redefinition is participating in the synchronization!

So what I would like to do is take your basic VolatileData part of the 
patch and run with it for JEP-149 purposes, while separating the 
annotations issue so they can be dealt with by the experts in that 
particular area.

I'm sorry it has taken so long to arrive at a fairly negative position, 
but I need someone else to take up the annotations gauntlet and run with it.

Thanks,
David

On 3/12/2012 5:41 PM, Peter Levart wrote:
> Hi David, Alan, Alexander and others,
>
> In the meanwhile I have added another annotations space optimization to
> the patch. If a Class doesn't inherit any annotations from a superclass,
> which I think is a common case, it assigns the same Map instance to
> "annotations" as well as "declaredAnnotations" fields. Previously - and
> in the original code - this only happened for java.lang.Object and
> interfaces.
>
> Here's the updated webrev:
>
> http://dl.dropbox.com/u/101777488/jdk8-tl/JEP-149/webrev.02/index.html
>
> I have also rewritten the performance micro-benchmarks. With the
> addition of repeating annotations, one performance aspect surfaces: when
> asking for a particular annotation type on a Class and that annotation
> is not present, the new repeating annotations support method
> AnnotationSupport.getOneAnnotation asks for @ContainedBy meta-annotation
> on the annotation type. This can result in an even more apparent
> synchronization hot-spot with original code that uses synchronized
> initAnnotationsIfNecessary(). This aspect is tested with the 3rd test.
> Other 2 tests test the same thing as before but are more stable now,
> since now they measure retrieval of 5 different annotation types from
> each AnnotatedElement, previously they only measured retrieval of 1
> which was very sensitive to HashMap irregularities (it could happen that
> a particular key mapped to a bucket that was overloaded in one test-run
> and not in another)...
>
> Here're the new tests:
>
> https://raw.github.com/plevart/jdk8-tl/JEP-149/test/src/test/ReflectionTest.java
>
> And the corresponding results when run on an i7 CPU on Linux:
>
> https://raw.github.com/plevart/jdk8-tl/JEP-149/test/benchmark_results_i7-2600K.txt
>
>
> Regards, Peter
>
>
>
> On 12/03/2012 02:15 AM, David Holmes wrote:
>> On 1/12/2012 4:54 AM, Alan Bateman wrote:
>>> On 30/11/2012 18:36, Peter Levart wrote:
>>>> :
>>>>
>>>> So, what do you think? What kind of tests should I prepare in addidion
>>>> to those 3 so that the patch might get a consideration?
>>> I think this is good work and thanks for re-basing your patch. I know
>>> David plans to do a detail review. I think it will require extensive
>>> performance testing too, perhaps with some large applications.
>>
>> Indeed I do plan a detailed review and have initiated some initial
>> performance tests.
>>
>> I am also swamped but will try to get to the review this week - and
>> will also need to check the referenced annotations updates.
>>
>> David
>>
>>> -Alan
>>>
>