On 08/13/2013 09:52 AM, Joel Borggrén-Franck wrote:
1) We should really measure this.
Hi Joel,
Here're some measurements of memory usages for some typical situations:
// no annotations
public static class *Unannotated* {
}
// typical annotations (JPA-like)
@Target(TYPE) @Retention(RUNTIME)
public @interface Entity {
String name() default "";
}
@Target(TYPE) @Retention(RUNTIME)
public @interface Table {
String name() default "";
String catalog() default "";
String schema() default "";
UniqueConstraint[] uniqueConstraints() default {};
}
@Target({}) @Retention(RUNTIME)
public @interface UniqueConstraint {
String[] columnNames();
}
@Entity(name = "EntityName")
@Table(
name = "table_name",
uniqueConstraints = {
@UniqueConstraint(columnNames = {"code"})
}
)
public static class *JpaEntity* {
}
// inherited annotations
@Target(TYPE) @Retention(RUNTIME)
public @interface PlainAnn {
String value();
}
@Target(TYPE) @Retention(RUNTIME) @Inherited
public @interface IngeritableAnn {
String value();
}
@PlainAnn("super")
@IngeritableAnn("super")
public static class *AnnotatedSuper* {
}
@PlainAnn("sub")
public static class *AnnotatedSub* {
}
I measured size of deep object graph starting with a j.l.Class object
once on fresh j.j.Class instance and once after getAnnotations() has
been called then both sizes were subtracted to get the size of
annotations only. The deep object graph ignored: cached Integer, Long,
Short, Byte, Character, Boolean, interned String, singletons such as
Collections.emptyMap() and accounted for aliases so that each object
instance was summed exactly once. The results are for 32 bit / 64 bit
object pointers:
Unannotated JpaEntity AnnotatedSuper AnnotatedSub
============== ============== ============== ==============
Original 88 / 128 1600 / 2584 1192 / 1912 744 / 1216
Patched 24 / 40 1408 / 2272 1000 / 1600 584 / 952
============== ============== ============== ==============
diff -64 / -88 -192 / -312 -192 / -312 -160 / -264
As can be seen, the savings in memory are present in each of the
measured situations. The Unannotated class space savings are important
for example in applications that scan classes for annotations. Most of
classes don't have annotations and overhead in such situations is
substantially reduced.
Regards, Peter
On 08/27/2013 03:00 PM, Peter Levart wrote:
Hi Joel and others,
Here's a 3rd revision of this proposed patch:
http://cr.openjdk.java.net/~plevart/jdk8-tl/AnnotationData/webrev.03/
I used LinkedHashMap for annotations in this one. It means that now
even .getAnnotations() are reported in "declaration order": 1st
inherited (includes overridden) then declared (that are not
overriding). For example, using @Inherited annotations A1, A2, A3:
@A1("C")
@A2("C")
class C {}
@A1("D")
@A3("D")
class D extends C {}
D.class.getAnnotations() now returns: { @A1("D"), @A2("C"), @A3("D") }
...
The LHM constructor uses the following expression to estimate the
initial capacity of the LHM:
3326 annotations = new LinkedHashMap<>((Math.max(
3327 declaredAnnotations.size(),
3328 Math.min(12,
declaredAnnotations.size() + superAnnotations.size())
3329 ) * 4 + 2) / 3
3330 );
I think this strikes some balance between effectivity and accuracy of
estimation. I could pre-scan the superclass annotations and calculate
the exact final size of the annotations Map before constructing it
though. Tell me if this is worth the effort.
I also created a test that tests 3 things:
- class annotation inheritance
- order of class annotations reported by .getAnnotations() and
.getDeclaredAnnotations() methods (although not specified, the order
is now stable and resembles declaration order)
- behaviour of class annotation caching when class is redefined
Regards, Peter
On 08/13/2013 09:52 AM, Joel Borggrén-Franck wrote:
Hi,
Comments inline,
On 12 aug 2013, at 14:40, Peter Levart <peter.lev...@gmail.com> wrote:
On 08/12/2013 12:54 PM, Joel Borggren-Franck wrote:
- annotation (@interface) declarations can themselves be redefined
(for example, defaults changed). Such redefinitions don't affect
already initialized annotations. Default values are cached in
AnnotationType which is not invalidated as a result of class
redefinition. Maybe it should be. And even if AnnotationType was
invalidated as a result of class redefinition, the defaults are
looked-up when particular annotations are initialized and then
combined with parsed values in a single values map inside each
annotation instance (proxy), so invalidating AnnotationType would
have no effect on those annotations.
3326 if (annotations == null) { // lazy construction
3327 annotations = new HashMap<>();
3328 }
I think this should be a LinkedHashMap, so that iteration is
predictable
(I know it isn't in the current code). Also the size of the map is
known
so you can use a constructor with an explicit initial capacity.
Right, AnnotationParser does return LinkedHashMap, so at least
decalredAnnotations are in parse-order. I will change the code to
use LinkedHashMap for inherited/combined case too.
Thanks.
The number of annotations that end-up in inherited/combined Map is
not known in advance. I could make a separate pre-scan for
superclass annotations that are @Inheritable and don't have the same
key as any of declared annotations and then sum this count with
declared annotations count, but I don't think this will be very
effective. I could allocate a large-enough Map to always fit (the
count of superclass annotations + the count of declared
annotations), but that could sometimes lead to much over-allocated
Maps. I could take the min(DEFAULT_CAPACITY, superclass annotations
count + declared annotations count) as the initial capacity for the
Map. What do you think which of those 3 alternatives is the best?
My bad. I was thinking of the case where every inherited annotation
was overridden, in that case annotations.size() ==
declaredAnnotations.size(). That is of course not generally true. I'm
fine with handling this as a follow up since the situation is no
worse than today and the surrounding code is better. However,
1) We should really measure this.
2) My gut feeling is that the ratio of not overridden inherited
annotations to declared annotations is small IE the expected nr of
annotations is much closer to declare annotations than to declared +
superclass annotations.
3) The default initial capacity is 16 and load factor is 0.75. I do
think the mean nr of annotations is closer to 3 than to 12. We are
probably wasting some space here.
Perhaps use min(default cap, (total annotations/0.75 (+ 1?))) for now
as that will make the situation no worse than today and often better?
Since this is a fairly significant rewrite I think it might be good to
make sure our tests exercise the new code. Can you run some kind of
coverage report on this?
I successfully ran the jdk_lang jtreg tests which contain several
tests for annotations.
I'm a bit worried these tests doesn't cover annotations + class
redefine. But I might be persuaded to have more extensive testing as
a follow up as well.
cheers
/Joel
