RFR: 8004643 Reduce javac space overhead introduced with compiler support for repeating annotations

Tue Jun 4 23:08:27 PDT 2013

Hi Jon,

I think it's great that all these allocations of Annotations are removed.
The changes look good to me.

cu, WMD.

On Tue, Jun 4, 2013 at 9:24 AM, Jonathan Gibbons <
jonathan.gibbons at oracle.com> wrote:

>  Thanks.   JDK-8015899 filed to track the folowup issue.
>
> I think we need to focus on consolidating the Annotations abstraction
> before we focus on cleaning up how it is accessed via Symbol.  Or at least,
> we should work on it in maybe multiple cleanup phases.
>
> --Jon
>
>
>
>
> On 06/04/2013 09:13 AM, Maurizio Cimadamore wrote:
>
> Hi Jon,
> the patch is fine in th short term - this is a list of issues I think we
> should work on to alleviate the pain in this area:
>
> *) There's an annoying split between symbol annos and type annos; we have
> twice the methods we need - I think some work should be done to consolidate
> existing APIs onto a more stable version where stuff like this:
>
> public boolean hasAnnotations() {
> +        return (annotations != null && !annotations.isEmpty());
> +    }
> +
> +    public boolean hasTypeAnnotations() {
> +        return (annotations != null && !annotations.isTypesEmpty());
> +    }
>
>
> Is replaced  by this:
>
> public boolean hasAnnotations(AnnoKind kind) { ... }
>
>
> *) I think all methods that modify annotations should not be exposed
> through Symbol API
>
> *) This should be done via a Symbol flag
>
> - annotationsPendingCompletion
>
> In other words, I would keep the Symbol API light by providing:
>
> *) a way for getting symbol/type annos
> *) a way to tell as to whether annotation completion has already happened
> (i.e. will the above method yield meaningful answer?)
>
> Everything else should be done through a separate class (Annotations),
> pretty much like we do now, by accessing symbol's metadata and playing with
> it. I also would remove any back pointer from Annotations to Symbol (if
> any).
>
> Maurizio
>
>
> On 01/06/13 03:14, Jonathan Gibbons wrote:
>
> More from the javac janitation crew, cleaning up the odds and ends.
>
> This is a relatively simple fix for the space overhead introduced by
> allocating an Annotations object for every Symbol.  The fix is to hide the
> Annotations object behind methods in Symbol (which also cleans up the
> client code a bunch) and to lazily allocate the Annotations object if and
> only if it is needed.  If the reference to the Annotations object is null,
> that means "no annotations on this symbol". (I tried using an immutable
> empty object, but it caused more clutter than it saved.)
>
> Most notably, Annotations is not itself changed by this changeset.
>
> The savings on a JDK build are significant, going from every Symbol having
> an Annotations object, to < 1% of Symbols needing an Annotations object ...
> from 4,024,455 to just 35,272.  In the table below, each row is a separate
> javac compilation in the JDK build.  The left hand block of numbers comes
> from a recent build of TL; the right hand block of numbers comes from a
> build of TL with the patch applied.  There may be more optimizations
> possible, by further improving the checks on whether a new Annotations
> object is required.
>
> There's a couple of related minor cleanups here as well. Going forward, it
> would be nice to clean up the method names on Annotations and the wrapper
> methods on Symbol. There's an inconsistent set of names using fragments
> like {(empty), Raw, Declaration, Type} x {Attributes, Annotations}.   Once
> we can decide on a better set of names, we can do a separate "renaming
> only" changeset for that.
>
> The webrev is available at
> http://cr.openjdk.java.net/~jjg/8004643/webrev.00/  It is against
> tl/langtools.
>
>
>
>
>
>
> -- Jon
>
>
>
>
>
>  OLD
>
>
>  NEW
>
>
>
>  Symbols Annotations %
>  Symbols Annotations %
>
>  269,144 269,144 100.00%
>  269,191 1,817 0.67%
>
>  4,463 4,463 100.00%
>  4,463 32 0.72%
>
>  39,845 39,845 100.00%
>  39,845 40 0.10%
>
>  2,276 2,276 100.00%
>  2,276 7 0.31%
>
>  224,546 224,546 100.00%
>  224,546 182 0.08%
>
>  356,137 356,137 100.00%
>  356,137 343 0.10%
>
>  8,610 8,610 100.00%
>  8,610 37 0.43%
>
>  428,097 428,097 100.00%
>  428,097 4,884 1.14%
>
>  49,031 49,031 100.00%
>  49,031 253 0.52%
>
>  1,338 1,338 100.00%
>  1,338 4 0.30%
>
>  2,016,962 2,016,962 100.00%
>  2,017,279 9,824 0.49%
>
>  2,642 2,642 100.00%
>  3,944 110 2.79%
>
>  3,937 3,937 100.00%
>  2,642 78 2.95%
>
>  3,944 3,944 100.00%
>  3,937 112 2.84%
>
>  4,015 4,015 100.00%
>  4,015 101 2.52%
>
>  5,361 5,361 100.00%
>  4,462 120 2.69%
>
>  4,779 4,779 100.00%
>  5,361 139 2.59%
>
>  3,855 3,855 100.00%
>  4,281 88 2.06%
>
>  4,281 4,281 100.00%
>  4,779 135 2.82%
>
>  4,462 4,462 100.00%
>  4,447 107 2.41%
>
>  4,546 4,546 100.00%
>  4,546 102 2.24%
>
>  5,795 5,795 100.00%
>  3,855 28 0.73%
>
>  4,447 4,447 100.00%
>  5,795 144 2.48%
>
>  5,176 5,176 100.00%
>  6,496 117 1.80%
>
>  6,496 6,496 100.00%
>  5,176 144 2.78%
>
>  6,056 6,056 100.00%
>  6,056 118 1.95%
>
>  5,759 5,759 100.00%
>  5,759 112 1.94%
>
>  4,926 4,926 100.00%
>  4,926 106 2.15%
>
>  1,025 1,025 100.00%
>  6,468 136 2.10%
>
>  3,697 3,697 100.00%
>  4,185 43 1.03%
>
>  6,468 6,468 100.00%
>  7,804 139 1.78%
>
>  4,185 4,185 100.00%
>  3,697 40 1.08%
>
>  7,809 7,809 100.00%
>  1,025 12 1.17%
>
>  8,878 8,878 100.00%
>  8,878 170 1.91%
>
>  7,804 7,804 100.00%
>  7,809 173 2.22%
>
>  1,029 1,029 100.00%
>  10,420 164 1.57%
>
>  1,403 1,403 100.00%
>  8,011 183 2.28%
>
>  10,420 10,420 100.00%
>  10,024 227 2.26%
>
>  8,011 8,011 100.00%
>  11,303 190 1.68%
>
>  10,024 10,024 100.00%
>  12,225 231 1.89%
>
>  11,303 11,303 100.00%
>  1,029 12 1.17%
>
>  12,225 12,225 100.00%
>  1,403 28 2.00%
>
>  15,827 15,827 100.00%
>  15,827 173 1.09%
>
>  12,642 12,642 100.00%
>  12,686 166 1.31%
>
>  124,653 124,653 100.00%
>  124,654 2,824 2.27%
>
>  39,797 39,797 100.00%
>  39,797 691 1.74%
>
>  255,866 255,866 100.00%
>  255,910 10,386 4.06%
>  TOTAL 4,023,992 4,023,992 100.00%
>  4,024,445 35,272 0.88%
>
>
>
>

-- 
http://www.google.com/profiles/wdietl
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/type-annotations-dev/attachments/20130604/a3289c05/attachment-0001.html 