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 198604 - LayerGeneratingProcessor keeps javac instances through originatingElementsByProcessor map
Summary: LayerGeneratingProcessor keeps javac instances through originatingElementsByP...
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Filesystems (show other bugs)
Version: 7.0
Hardware: PC Linux
: P2 normal (vote)
Assignee: Jesse Glick
URL:
Keywords: PERFORMANCE
Depends on:
Blocks:
 
Reported: 2011-05-14 11:59 UTC by Jan Lahoda
Modified: 2011-08-14 07:26 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Same patch but as an attachment so as not to lose formatting (1.33 KB, patch)
2011-05-16 22:10 UTC, Jesse Glick
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Lahoda 2011-05-14 11:59:22 UTC
[recent sources]

I was editing a hint, and after a while the IDE got almost out of memory. While inspecting the heap dump, I have found that there are 55 instances of javac on the heap. These instances appear to be held through the LayerGeneratingProcessor.originatingElementsByProcessor. This map holds Elements in its value, and some of these elements are transitively holding the ProcessingEnvironment, the WeakHM key. So the mapping cannot be cleared from the Map. The LGP will not clear the map if any error has been raised during processing. I would suggest to always clear this (and the other LGP map).
Comment 1 Jesse Glick 2011-05-16 22:08:16 UTC
Are you able to reproduce? If so, please try the following patch. If it works (or at least seems to do no harm), I will commit it on Thursday (and this might be a 70patch1-candidate):

diff --git a/openide.filesystems/src/org/openide/filesystems/annotations/LayerGeneratingProcessor.java b/openide.filesystems/src/org/openide/filesystems/annotations/LayerGeneratingProcessor.java
--- a/openide.filesystems/src/org/openide/filesystems/annotations/LayerGeneratingProcessor.java
+++ b/openide.filesystems/src/org/openide/filesystems/annotations/LayerGeneratingProcessor.java
@@ -134,11 +134,11 @@
             }
             return false;
         }
-        if (roundEnv.processingOver() && !roundEnv.errorRaised()) {
+        if (roundEnv.processingOver()) {
             Document doc = generatedLayerByProcessor.remove(processingEnv);
-            if (doc != null) {
+            List<Element> originatingElementsL = originatingElementsByProcessor.remove(processingEnv);
+            if (doc != null && !roundEnv.errorRaised()) {
                 Element[] originatingElementsA = new Element[0];
-                List<Element> originatingElementsL = originatingElementsByProcessor.remove(processingEnv);
                 if (originatingElementsL != null) {
                     originatingElementsA = originatingElementsL.toArray(originatingElementsA);
                 }

Of course I would prefer to make the maps intrinsically safe from leaks rather than rely on the processor being called with roundEnv.processingOver(), but that seems impossible without Java #6389107.
Comment 2 Jesse Glick 2011-05-16 22:10:51 UTC
Created attachment 108334 [details]
Same patch but as an attachment so as not to lose formatting
Comment 3 Jan Lahoda 2011-05-17 15:45:32 UTC
Thanks. I guess it will be enough to edit a hint and check the number of javac instances - I will the testing soon.
Comment 4 Jaroslav Tulach 2011-05-19 15:05:31 UTC
A reference to a JavaC created by Java.Source+Parsing.API is a good candidate for showing in Timer&Counters window, I think.
Comment 5 Jan Lahoda 2011-05-19 15:22:59 UTC
The patch fixes the problem for me, thanks.

For files that are opened in the editor, we register their CompilationUnitTree in timers&counters. This works fine for things related to editing, but obviously not as a global status. I will try to add it to the global status too.
Comment 6 Jesse Glick 2011-05-19 21:22:42 UTC
With test: core-main #5001a7862f5e
Comment 7 Quality Engineering 2011-05-20 08:54:14 UTC
Integrated into 'main-golden', will be available in build *201105200400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/5001a7862f5e
User: Jesse Glick <jglick@netbeans.org>
Log: #198604: memory leak due to WeakHashMap values -> keys.
Comment 8 Jaroslav Tulach 2011-05-20 12:01:13 UTC
Something is wrong on MacOSX:

http://hudson4qe.czech.sun.com/job/platform-filesystems/jdk=JDK 1.6,label=OSX/lastCompletedBuild/testReport/org.openide.filesystems.annotations/LayerGeneratingProcessorTest/testProcessingEnvironmentLeak/?

junit.framework.AssertionFailedError: can collect ProcessingEnvironment:
public static final com.sun.tools.javac.code.Type com.sun.tools.javac.code.Symtab.byteType->
com.sun.tools.javac.code.Type@7a3a30-tsym->
com.sun.tools.javac.code.Symbol$ClassSymbol@354093-owner->
com.sun.tools.javac.code.Symbol$PackageSymbol@14bcb5c-completer->
com.sun.tools.javac.jvm.ClassReader@b6be69-sourceCompleter->
com.sun.tools.javac.main.JavaCompiler@121f4ef-context->
com.sun.tools.javac.util.Context@1464fee-ht->
java.util.HashMap@16f2b7f-table->
[Ljava.util.HashMap$Entry;@b536e6-[58]->
java.util.HashMap$Entry@1d9626b-value->
com.sun.tools.javac.api.JavacTaskImpl@22023a-processors->
java.util.Collections$SingletonSet@6bb78a-element->
org.openide.filesystems.annotations.LayerGeneratingProcessorTest$P@1e31f72-env->
com.sun.tools.javac.processing.JavacProcessingEnvironment@1b1fbf4
        at junit.framework.Assert.fail(Assert.java:47)
        at org.netbeans.junit.NbTestCase$4.run(NbTestCase.java:1361)
Comment 9 Jesse Glick 2011-05-20 14:40:57 UTC
(In reply to comment #8)
> Something is wrong on MacOSX

Apparently so, but it is not the same reference chain as is implicated in this bug, so reclosing. I am not sure what the quoted chain means. The test has been run just once on each machine, so it is not yet possible to say that the failure is Mac-specific (especially as I have no access to a Mac to test on), but it could be a bug in some older (?) version of javac included in the Mac JDK. If the failure persists on Mac I will just disable the test on that platform, I suppose.

Backporting to 7.0.1: releases #b73e4016ae91
Comment 10 Jesse Glick 2011-05-20 14:46:54 UTC
(In reply to comment #9)
> it could be a bug in some older (?) version of javac included in the Mac JDK

In particular,

public static final com.sun.tools.javac.code.Type com.sun.tools.javac.code.Symtab.byteType

is suspicious, since both JDK 6u22 and OpenJDK 7 sources list this field as nonstatic. Even Linux JDK 5u22 has it as nonstatic. Perhaps someone with a Mac could confirm that a nonstandard version of javac is bundled.
Comment 11 Jan Lahoda 2011-05-20 15:00:28 UTC
Older versions of javac had several static fields in Symtab, that kept an instance of javac in memory as long as Symtab class was held. Maybe the test could check if "com.sun.tools.javac.code.Symtab.byteType" exists and is static and stop if both these conditions are met?
Comment 12 Jaroslav Tulach 2011-05-22 19:20:33 UTC
Probably the simplest way to make the test pass on Mac. I'll find an Mac owner and force him to do the fix.
Comment 13 Jan Lahoda 2011-05-22 20:26:49 UTC
I do not think this has much to do with Mac - all that is needed is sufficiently old JDK. I was able to reproduce the test failure on Linux with:
$ java -version
java version "1.6.0-rc"
Java(TM) SE Runtime Environment (build 1.6.0-rc-b104)
Java HotSpot(TM) Server VM (build 1.6.0-rc-b104, mixed mode)

But I am quite sure that the test will also fail on 1.6u05 (untested). The test will most likely pass on update 14 (I do not have any updates between these two). So it should be easy to create and test the workaround on Linux.
Comment 14 Jesse Glick 2011-05-24 22:27:56 UTC
Indeed fails for me in JDK 6[u0] on Linux. Working around it: core-main #113d79747ad9