This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 48960 - [perf] AST cache too big
Summary: [perf] AST cache too big
Status: RESOLVED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: Unsupported (show other bugs)
Version: 4.x
Hardware: All All
: P2 blocker (vote)
Assignee: issues@java
URL:
Keywords: PERFORMANCE
Depends on: 49414
Blocks: 43258
  Show dependency tree
 
Reported: 2004-09-13 21:30 UTC by Petr Nejedly
Modified: 2007-09-26 09:14 UTC (History)
4 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Nejedly 2004-09-13 21:30:29 UTC
It seems that the Name$Table leak has reappeared
lately. After mounting web/project and looking for
direct subclasses of HelpCtx.Provider (and closing
results), there were additional 2MB in byte[]s,
2MB in Name[]s.

16 Name[] are referenced through:
org.netbeans.modules.javacore.parser.ElementInfo$CachedReference.CACHE->
org.netbeans.modules.javacore.parser.ElementInfo$1@16fa->
[Ljava.util.HashMap$Entry;@3353->
java.util.LinkedHashMap$Entry@5c91->
org.netbeans.modules.javacore.parser.MDRParser@c5d8->
org.netbeans.lib.gjast.ASTopLevel@17f46->
com.sun.tools.javac.util.Name@28ad6->
com.sun.tools.javac.util.Name$Table@414ec->
[Lcom.sun.tools.javac.util.Name;@61a2c

(different instaces down from the ...$Entry)

Another 16 are held through:
org.netbeans.modules.javacore.parser.ElementInfo$CachedReference.CACHE->
org.netbeans.modules.javacore.parser.ElementInfo$1@16fa->
[Ljava.util.HashMap$Entry;@3353->
java.util.LinkedHashMap$Entry@5c9d->
org.netbeans.modules.javacore.parser.MDRParser@c5e4->
org.netbeans.lib.gjast.ASTopLevel@17fee->
org.netbeans.lib.gjast.ASParser$BridgeContext@2f592->
org.netbeans.lib.gjast.ASScanner@4a1bd->
com.sun.tools.javac.parser.Keywords@6d406->
[Lcom.sun.tools.javac.util.Name;@93c72
Comment 1 Tomas Hurka 2004-09-14 09:59:22 UTC
Definitely not P2 issue. There is a cache 
(org.netbeans.modules.javacore.parser.ElementInfo.CachedReference.CACHE) , which 
holds last 16 AST's. So I don't think that there is memory leak in MDRParser. Can you 
please describe, what is wrong. Thanks.
Comment 2 Petr Nejedly 2004-09-14 22:39:32 UTC
Last time by the issue 43258, it was claimed there is only one
Name$Table shared, so I took that claim as an etalon. Here I see 32 of
them, 16 being large (and I somehow intuitively expect that those
smaller 16, Keywords, will in fact be exact duplicates).

Anyway, you have a cache with 16 slots, each slot consuming nearly a
megabyte. For such a big cache, you should have pretty clear
justification of the size of the cache (16) and the number shouldn't
be chosen arbitrarily. Do you have hit/miss distributions for
different workloads and different cache sizes?

For my two tested workloads (find direct subclasses and find all
subclasses of HelpCtx.Provider), the cache has practically no effect.
I've tried sizes from 2 to 32 and the times to finish the queries were
about the same*, so for this workload, 16 slots is way too much.

*) My measurements may be influenced by the MDR storing features, so I
may be wrong when it comes to real parsing. But even in that case, the
MDR should work as good cache.

Finally, I was criticizing the Name$Table impl before, that it uses
big preallocated number of slots that consume a lot of memory if used
as a long living structure (e.g. in cache). E.g. for 16 cache slots,
there are 500.000 Name slots in 16 tables, while typically only about
20.000-30.000 of them collectively are used. 
Comment 3 Martin Matula 2004-09-24 11:35:32 UTC
Determining ideal size of the caches is covered by issue 49414. So if
I understand it correctly, this issue is about big number of
pre-allocated Name slots in case of bigger AST cache? Note that the
ASTCache size may be significantly lowered as a result of resolving
issue 49414. CC'ing Tom in case you still want to push for some
changes in names caching and lowering the priority.
Comment 4 _ ttran 2004-09-24 13:26:04 UTC
Martin, I think Petr N made it very clear in his comment.  He thought
this is a leak or a serious misuse of the cache.  I don't know if he's
right or wrong.  But until it's resolved, this remains a serious issue.

-> P2
Comment 5 Martin Matula 2004-09-24 14:45:12 UTC
OK, so let me summarize the issue again so that we understand what
actions we need to take to resolve it:
1) Too much memory is consumed by AST cache, which could possibly be
made smaller -> its size will be directly determined by resolving
issue 49414
2) There is no leak (at least by the definition that I know)
3) Name$Table does not seem to be shared as claimed in issue 43258 -
Tom, could you please shed more light into this?
4) Name$Table uses big preallocated number of slots that consume a lot
of memory - Tom, could this be fixed?

Anything I missed?
Comment 6 _ tball 2004-09-24 18:25:24 UTC
The shared name table was removed due to a complaint that it was
static and took too much space (even though it is smaller than the sum
of the individual ones).  There must be one name table for each
top-level AST, so the number of name tables is determined by how many
AST structures are cached.  

I now use javac's name table pooling, so SoftReferences of the
released tables are kept in a list for future use.  This improves
performance since many names are shared between compilations and so do
not need to be re-initialized.

I can certainly make the tables smaller, as we are currently using
javac's default values.  
Comment 7 _ tball 2004-09-25 02:24:14 UTC
Reduced name table size as requested.  AST cache adjustment needs to
be done by someone else (two separate issues next time would be nice!).
Comment 8 Petr Nejedly 2004-09-30 09:23:31 UTC
> The shared name table was removed due to a complaint that it was
> static and took too much space
I thought we agreed that one shared is better in the issue 43258
(my comment from 2004-09-03).

Anyway, that table was typically consuming about 1MB, while those 16
tables held by 16 ASTs consume together only 0.5MB for me, so I
consider this state better (without knowing other perf. implications).

Comment 9 _ tball 2004-09-30 17:02:45 UTC
My commit later today (after testing) should reduce the importance of
this issue.  Each AST currently has a reference to the javac build
that created its trees; this happens because even though the interface
between javac and javacore doesn't include any javac references, the
implementation of that interface does in gjast.  

Assuming testing finishes successfully, today's change will copy the
generated tree into a javac-free set, as well as separating the
JParser implementation from the compiler context it used.  This means
that no references remain between the trees, the tokens, and the
engine that created them.  This should make the AST cache much smaller.
Comment 10 Martin Matula 2004-10-20 16:32:49 UTC
The AST cache size was set to 2 and the cache overhead itself was
improved by using an array instead of LinkedHashMap which is cheaper
for such a small size of the cache.
Comment 11 Jiri Prox 2005-07-12 11:30:30 UTC
Petre, can you, please, verify this issue? Thanks.
Comment 12 Quality Engineering 2007-09-20 12:11:35 UTC
Reorganization of java component