Request for review: 8004698: Implement Core Reflection for Type Annotations (original) (raw)

Peter Levart peter.levart at gmail.com
Tue Jan 22 17:01:04 UTC 2013

On 01/22/2013 01:47 PM, Joel Borggrén-Franck wrote:

Hi Peter,

Thanks for your comments, see inline, On 01/19/2013 06:11 PM, Peter Levart wrote:

I see, there is a dilema how to cache type annotations. To satisfy single-annotation-returning methods and repeating annotation logic, a HashMap would be a logical choice, but it has much bigger footprint than simple arrays of annotations...

I don't prioritize footprint for classes that have runtime visible type annotations. Those classes should be very few, and as a user you are making an explicit choice of adding metadata when you use runtime visible (type) annotations. So here is my list of priorities (and by un-annotated I mean without runtime visible annotations): - Unannotated classes/fields/methods should have as small as possible extra footprint, this is most important.

Hello Joel,

I imagine there will be additional places where references to Map<Class<? extends Annotation>, Annotation> will be added to hold cached annotations. To satisfy your priority #1 I would consistently make sure that when there are no annotations to put into the map, a singleton Collections.emptyMap() reference is put in place. This is currently not so for regular annotations (should be corrected). I know of runtime tools that "scan" classes for particular annotations, just to find out that majority of them are without annotations. If empty instances of HashMap(16) pop-up as a result of such scan, lots of memory is wasted.

- Classes/fields/methods without type annotations should have negligible footprint over classes/fields/methods with only regular annotations.

You meant "Classes/fields/methods with type annotations should have negligible footprint over classes/fields/methods with only regular annotations", right?

Regards, Peter

I experimented with an idea of using a special purpose immutable Map implementation:

https://github.com/plevart/jdk8-tl/blob/anno-map/jdk/src/share/classes/sun/reflect/annotation/UniqueIndex.java

and: https://github.com/plevart/jdk8-tl/blob/anno-map/jdk/src/share/classes/sun/reflect/annotation/AnnotationMap.java This is just a preview. I still have to create some tests to see how it compares to HashMap. Being an immutable wrapper for a single array of annotations it has a nice feature that a reference to it can be passed to other threads via a data race with no danger of data inconsistency, which simplifies caching mechanisms and helps scalability. Might even be a space-saving replacement for existing HashMaps of annotations. Depending on performance, of course. What do you think? Is it worth pursuing this further? I'd be very hesitant to introduce a new hashmap like type. Especially since IMHO footprint when we actually have annotations isn't as critical as when we are annotation free. My initial approach is probably to dust of your patches for an annotations cache on Class and adopt it for type annotations, and use on Field/Method/Constructor/(TypeVar). Thank you again for your work on this! cheers /Joel Regards, Peter On Jan 17, 2013 5:24 PM, "Joel Borggrén-Franck" <joel.franck at oracle.com_ _<mailto:joel.franck at oracle.com>> wrote: Hi, Here is a webrev for core reflection support for type annotations: http://cr.openjdk.java.net/~jfranck/8004698/webrev.00/ This code is based on the tl/jdk repository, but in order to run the tests you need a javac with type annotations support and also a recent copy of the VM that includes this change set: http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/rev/35431a769282 (This also means this code can't be pushed until all supporting pieces are in place.) Type annotations are still a bit of a moving target so there will be follow up work on this. There is no caching of annotations in this version and I am not sure how we want to do the space/time trade of for type annotations. cheers /Joel

More information about the core-libs-dev mailing list