RFR JDK-8011940 : java.lang.Class.getAnnotations() always enters synchronized method

[Joel Borggrén-Franck]

Hi all,

Comments inline,

On 12 aug 2013, at 15:19, Paul Sandoz <paul.sandoz at oracle.com> wrote:
> On Aug 12, 2013, at 2:40 PM, Peter Levart <peter.levart at gmail.com> wrote:
>> On 08/12/2013 12:54 PM, Joel Borggren-Franck wrote:

>>> I realize the interaction between probably any reflective operation and
>>> a redefine is blurry, but if a redefine occurs between line 3308 and 3309
>>> annotationData() will return a possibly outdated AnnotationData.
>>> 
>>> 3307             if (Atomic.casAnnotationData(this, annotationData, newAnnotationData)) {
>>> 3308                 // successfully installed new AnnotationData
>>> 
>>> redefine here
>>> 
>>> 3309                 return newAnnotationData;
>>> 3310             }
>>> 
>>> I suppose there is a sequencing of events where this will be inevitable,
>>> but this got me thinking about the state model of annotations (and
>>> reflective objects) through an redefine.
>> 
>> The AnnotationData created and returned concurrently with class redefinition can contain old or new version, yes, but that's not a problem, since the AnnotationData also contains a "redefinedCount" field, so any further requests for annotations will trigger re-computation. This assumes that VM either changes the state returned by getRawAnnotations() and "classRedefinedCount" field atomically (during a safepoint?) or at least in order: 1st getRawAnnotations(), 2nd "classRedefinedCount". We 1st read "classRedefinedCount" and then
>> getRawAnnotations(), so there's no danger of keeping and returning stale data after redefinition.
>> 
>> This is more or less the same as with ReflectionData caching.
>> 
> 
> I suppose one could do the cas without checking it's result:
> 
> 3296     private AnnotationData annotationData() {
> 3297         while (true) { // retry loop
> 3298             AnnotationData annotationData = this.annotationData;
> 3299             int classRedefinedCount = this.classRedefinedCount;
> 3300             if (annotationData != null &&
> 3301                 annotationData.redefinedCount == classRedefinedCount) {
> 3302                 return annotationData;
> 3303             }
> 3304             // null or stale annotationData -> optimistically create new instance
> 3306             // try to install it
> 3307             Atomic.casAnnotationData(this, annotationData, createAnnotationData(classRedefinedCount));
>                 // re-check conditions in case a re-defintion occurred while creating
> 3311         }
> 3312     }
> 
> Don't really know if it is worth it though, plus it might be better to break out the loop once set. Note frameworks, such as JAXB and JAX-RS will cache results of processing reflection and annotation information and need to be given a kick to re-process (e.g. like from JRebel).

I would guess it is not worth it. There still exists a sequencing of events such that the AnnotationData will be stale, the redefine just needs to happen after the return in this case. I suspect this is a fundamental issue with having reflective object that are not immutable. I think the fundamental problem here (which has nothing to do with Peters work) is that in general developers' mental model of core reflective objects is that they are immutable and suitable for caching. With class redefine that is not true and to make matters worse, we don't have a good story for "cache invalidation" of reflective objets. Again, this has nothing to do with Peters work.

cheers
/Joel