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" <[email protected]
<mailto:[email protected]>> 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
