Bug 123003

I think it was yesterday that I went almost all day without restarting NetBeans. I was editing plain .java files and
redeploying to GlassFish v2 many times.  It didn't give me any OutOfMemoryExceptions.  I didn't open the visual designer
during that period of time.

Is this really a duplicate of 122902?

Maybe, but in 122902 it seems the priority was knocked down to a P3, which I do not agree with. If there is a memory
leak and it makes something unusable then it is definitely a P1 as there is no workaround and it is very noticeable. I
point to . Ryan has increased his memory and been through the trials and errors, and in this issue he has a
reproduceable scenario. I point to:
http://qa.netbeans.org/bugzilla/bug_priority_guidelines.html

Which clearly states this to be a P1. It might be better to keep this issue and work on it until it is clearly
determined to be the other.

Well, I don't know how to address this. I guess the project is the same as it is in the issue #122902. I decreased it
because you can workaround it by bumping up the memory.
I am going to play with the project to try to find out the memory leaks.

The project attached to the other ticket is a completely blank new project created by NetBeans 6. I have not done
anything to it. I experienced that bug using a new project.  I'll look into creating a memory dump for you guys.  If
that doesn't help then I can probably give you (via direct email) several sample JSP/page beans/CSS/images from the main
project that causes NB to run out of memory frequently.

OK,I see, what about this project. Is it possible to attach it here? If not, just please describe roughly, how many
pages it has, whether it uses databases (etc.). Thanks.

So far I inspected the heap2.bin a bit. There were 71 instances of WebForm(designers), while it was supposed to be there
about 30 (or none, if all was closed and GC'ed). I went to closest GC root of one of the instances, and it pointed out
that there was still FileChangeListener on MasterFileSystem. The FileChangeListener was of FacesModelSet type (the
superclass ModelSet implements the FileChangeListener).
It seems that the destroy was not called on that FacesModelSet instance.
Anyway, I suggest to use weak listener instead relying on the destroy method call. Passing to insync. Tomorrow I am
going to investigate the other heap dump, and more instances to find whether there are other roots.

Good job! Regarding the 70 instances of WebForm, there should have been only one.  I had closed all pages, did the first
heap dump, then opened one page before doing the second heap dump. 

FacesModelSet is destroyed only when the project is closed. If the project is not closed the FacesModelSet is supposed
to live in the memory (at least as per the current impl). So this  is not a memory leak.

Was the Project (not the page) closed when the heap dump was taken.

Also what were the other GC Roots?

No the project was not closed

I'm digging down in the sources to see what FacesModelSet.java does and holds, but how much memory should each be
expected to consume? What is it based on? Certainly it will depend on the number of variables in a page etc, but I may
be wrong, but this is what it looks like. Should there be one or more for a single page (wondering why he had 70 of them
for 30 pages)? 

It looks like a lot of information is held just looking at the code for the first time, and I mean at first glance it
looks like each page has its own mock servlet/jsp container etc with UIView. I'm not sure what is released depending on
state and what is held in memory indefinitely though. Back to the 70 to 30 pages question (part for Ryan and part for
Sandip): Ryan, how many pages did you actually edit versus have open? What was the exact work flow? Sandip, just
guessing at the code, but a FacesModelSet should be reused upon a pages closing and opening, is this correct? I'm
looking at ModelSet.getModelSet, and if this were not happening wouldn't this be a memory leak?

It looks like a better approach, and again this is with a limited view and time thus far to review the code, would be to
have a stale phase (maybe this is already being done??) or round robin a limited number of project pages depending on
available heap. So, if the user doesn't open the file again for a couple minutes the memory is completely freed or there
is just a limit for the number kept in memory so a trade off between user experience performance and available memory
exists. I assume some of this hold over is part of the speed up issues - performance tiger team maybe (again not enough
information to know)??

When you asked me what the workflow was, were you asking what I did?  Basically I opened netbeans, then opened and
closed random pages over and over and over and over again, sometimes switching to java or jsp view and back to design
view, closed it, opened an other page, closed it, etc.  It took about 10 minutes or so.  I didn't edit any screen or
file, and did not press the save button for anything.

1 VW Project has one FacesModelSet which creates on FacesContainer (The mock container) and one FacesContext. Once
created the Faces

FacesModelSet holds the 1 FacesModel for each VW Page and VW ManagedBeans (subclasses of
Abstract(Request|Session|Applicatiopn)Beans.

FacesModelSet is lazily created on first request for modeling services from Insync. It sticks around till the project is
closed (unless there is a memory leak). The FacesModelSet's destroy method is supposed to do all the cleanup cascading
through all the FacesModels and *Units.

1 VW Page (or Fragment) has 1 FacesModel which has reuses one FacesContext from above. The FacesModel for a VW Page (or
Fragment) also has 1 UIViewRoot that corresponds to the JSF component tree root.

1 FacesModel has 1 LiveUnit which has 1 FacesPageUnit which has 1 MarkUpUnit and 1 JavaUnit. The MarkupUnit represents
the JSP source and associated datastructures. The JavaUnit represents the Java source. The JSP
source+document+dataobject brings all the datastructure associated with it. Same things goes for Java.

This is the Insync object structure in a nutshell. This has been the Insync architecture since the Creator days.

IMHO, at this time, we need to concentrate on using tools such as profiler to indicate where memory leaks are and then
look at the code for how to fix it.

The FacesModelSet leak pointed to by Peter was a false alarm because as pointed out by Ryan he had not closed the Project.

Thanks Sandip. That helps a lot understanding the order of things in VW. I just noticed I read wrong and had thought
there were multiple instances of FacesModelSet when it was actually WebForm...apologies there...but I am glad to have
the rundown on the lifetime and state of the objects.

I inspected so far the only one instance (the oldest one), which I supposed its project was closed. Anyway, it led to
the GC root of FacesModelSet, which contained the corresponding FacesModel, which is JsfForm hooked on to, and
consequently WebForm.

I am going to investigate the other instances and their roots. I'll let you know the results.

There is the question, when that happened (heap2.bin dump), how many pages were open, also how many projects were open
closed? Thanks.

Peter are you saying that the:

1. FacesModelSet was there even for a project that was closed?
2. There is a reference from FacesModel (which is refered to from the above FacesModelSet above) to JsfForm? Is it a
listener?

BTW I am having trouble downloading the heap dump from Ryan's website. It is very very slow. In fact it aborts in the
middle (happened twice).

About the download site not working very well, that is odd because we are on fiber.  However we do have a lot of inbound
traffic during the day.  Maybe I can upload these files for you somewhere else? Let me know where.

Regarding open pages: there was only one open page at the time I created heap2.bin.  It didn't fully open, it showed a
grey screen instead of my page, was using 99% cpu for many minutes and was locked up. 

There are 3 visual web project listed in the projects pane on the left.  I did not closed any visual web projects since
I opened NetBeans IDE to re-create the problem and dump the heap.  All I did was open the IDE, open and close many pages
from one of the projects many many times until it ran out of memory. It is possible that I opened some pages from the
other Visual Web project by accident since they have similar names... one targets desktop browsers, the other is for
mobile browsers.

Investigating further the heap dump, shows that there were 71 WebForm(designers), but only 30 FacesModels (representing
the corresponding models).
So far I investigated 2 instances of JsfTopComponent (1:1 relationship to WebForm), and both let to the FacesModelSet
via FacesModel FacesPageUnit MarkupUnit then source or rendered document and mappings from document/elemnets to boxes.

Now it seems to me that the FacesModel got reused (not garbaged after first close), but there remained the old
documents, which held onto (via the user data mappings) onto the WebForm(designer).

Now I am thinking that either the closed documents were not cleared, or if they don't need to. Some of the mappings to
switch from hard refs (old impl) to weak refs.

There are many mappings (it is not possible from the profiler to find out which all are in play), but I have an idea at
least in one.

The one which maps boxes to elements, to change from hard refs (of the boxes) to weak refs.

I am not sure that will be sufficient (more mapping might be needed to change similar way), but it shouldn't harm.

What do you think so far?

I profiled a similar test case (repeatedly opening and closing a single page) and got the same result.  The user data
for the current RaveSourceDocument has references to all the previously opened JsfTopComponents (PageBox -> JViewPort ->
JScrollPane -> JsfTopComponent).

Created attachment 53694 [details]
Potential fix (trunk), which would map boxes to elements weakly.

Peter's last fix did not seem to resolve the issue fully. There seem to be more references to JsfTopComponents
(WebForms) still hanging around. Peter is investigating.

Provided fix in designer module (for trunk).

Note:
The refined scenario is:
1) Open VW project
2) Open designer
3) Close the designer
4) Reopen the designer
There use to linger two instances of the JsfTopComponent

Now the older one is allowed to be garbaged.
The test, be aware to press the GC more often to get rid of the old ref. Also you may not switch to jsp or java tab
during the designer is opened. I am going to check whether there is remaining issue with them.

Fix in trunk:
Checking in visualweb/designer/src/org/netbeans/modules/visualweb/designer/WebForm.java;
/cvs/visualweb/designer/src/org/netbeans/modules/visualweb/designer/WebForm.java,v  <--  WebForm.java
new revision: 1.177; previous revision: 1.176
done

There is also one more condition needed (in the last comment). Also collaps the Page node (to be not visible in the
explorer). I am investigating that part now.

With the node open, it is OK, there seems to be MainWindow (via inputContext) holds onto the last component (via
InputMethodContext impl).

When opening more times the component, and invoking GC (why it doesn't work automatically?), then the older instances
disappear. (providing there was no switch to jsp or java, investigating those).

Calling GC is just telling to free what it can then. If it is going away with a GC it will eventually go away
automatically, but it will only do so once enough memory is used and the JVM sees it needs to reclaim it. It is a
performance game with the collector.

Yes, I just meant that the GC somehow is not triggered automatically in the IDE (one need to press the GC in the memory
toolbar). OK that is other issue probably.

So I investigated the cases when also JSP and Java gets open, and there always leak 2 instances but not more (one was
somehow kept by InputMethodContext from MainWindow, that I discovered mostly, and once I saw also SourceMonitor (in
insync), and EditorRegistry (openide or core?). But not more instances where leaking.

I think it is quite an improvement, so I am marking this as fixed in trunk, and going to look whether it is still
possible to integrate into 60 branch.

Created attachment 53730 [details]
Diff of fix for release60 branch

Now I need to reopen the issue, per Ryan's try (see the heap dump at http://www.ijws.com/~ryan/netbeans/patched/).

His scenario:

1) He kept one project open
2) and kept opening 5 designer pages, while also switching to jsp and java tab, and then closing them
3) He went all the way trhough entire project, and then again from the beginning
4) When he was about in the middle of the pages it the OutOfMemory happened.

I investigated it, and found another leak. It is SourceMonitor (there were 22 instances, while mostly 5 should be
there). Looking into the code reveals the problem.
The SourceMonitors are not removed from the map (they are mapped to the FacesModel).
Passing to insync now, this part needs to get fixed as well.

To further explain, the SourceMonitor held text document (jsp or java), which holds editor instances, which hold editor
cookie imp, which holds multiview element, which hold entire multiview component, and that also holds the designer.

Investigating...

So far according to my tests SourceMonitor is not leaking with a simple one page project with repeated open/close of the
page with switching to DESIGNER/JSP/JAVA. Will try with larger project.

It is leaking, as described. Even when opened/closed the same page 3 times. There is one SourceMonitor, which is holding
the chain which leads to the 3 designers (the SourceMonitor holds the same text document, but it leads to 3 different
jsf multiview elements).

Probably it (the chain) could be 'cut' somewhere else (if the source monitor is needed).

Well, guys, he problem in Ryan's heap dump is completly different and quite apparent.
The biggest consumer of the memory there is a little typo in:
/web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/NavigationRuleImpl.java
CCing author of the problem.
As the issue is really serious, I'll attach a patch (untested), that should reduce the memory usage for Ryan's case by
130MB, maybe more. I found that the same problematic pattern is spread all over the package, so the patch fixes all
cases I found.

Created attachment 53868 [details]
A patch reducing memory usage (and an actual leak of support data) significantly

Of course I'm not saying that the SourceMonitor(+ related) leak should not be fixed as well ;-)
Just that the static lists in the facesmodel/*Impl classes can grow pretty quickly even while editing single page.

Will this fix make a noticeable performance improvement?  As it is (without the fix), it feels like any time I want to
open a page to edit, I have to wait 10+ seconds.  To close the page I sometimes wait 30+ seconds.  I rarely close then
re-open the same file, so I always feel held back by Visual Web. It would be great if this is the source of the
performance issues.

I fixed the facesmodel part in trunk:
web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/DescriptionGroupImpl.java,v1.4
web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/IconImpl.java,v1.3
web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/LocaleConfigImpl.java,v1.3
web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/ManagedBeanImpl.java,v1.8
web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/NavigationCaseImpl.java,v1.7
web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/NavigationRuleImpl.java,v1.6
web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/ResourceBundleImpl.java,v1.3

The rest of the leak remains.
I would strongly vote for including this part of the fix to patch1, as it would improve
the user experience with visual web significantly.

The issue fixed by Petr Nejedly is tracked in the issue #123427. the remaining leaks should be resolved in this issue.

In the mean time I fixed another leak (found by Sandip) in OutlinePanelProvider.

Checking in visualweb/outline/src/org/netbeans/modules/visualweb/outline/OutlinePanelProvider.java;
/cvs/visualweb/outline/src/org/netbeans/modules/visualweb/outline/OutlinePanelProvider.java,v  <-- 
OutlinePanelProvider.java
new revision: 1.8; previous revision: 1.7
done

The JavaSource.editorRegistryListener field contains a leaked reference to a single JsfTopComponent (in addition to the
SourceMonitor leak).

There also seems to be a memory leak in RaveRenderedDocument.  The userData map for one RaveRenderedDocument contains
references to all the other RaveRenderedDocument objects.  This causes a single leak from
JavaSource.editorRegistryListener to retain references to the other previously opened pages (including the corresponding
FacesModels, etc.).

The objects cached in the userData for RaveRenderedDocument should be limited, and/or the JsfTopComponent fields should
be unset when the JsfTopComponent is no longer in use.

To Quy's comment.
The JsfTopComponent is OK. The thing is that nobody should hold the JsfTopComponent after it is used, not that the
JsfTopComponent should clear something, the component simply shouldn't be kept in memory at all. I.e. you need to
investigate who holds the component, not what the component holds.

The RaveRenderedDocument note might be relevant.
The RaveRenderedDocument points through the userData (among others) to RaveSourceDocument (or vice versa, or both), as
its JSP source. I think both of those should be also garbaged, but I am not sure about this one.
Sandip, is the source document still held by insync?
In any case, the mapping could be made weak, and I need to check all that, thanks.

Curently I am working on handling the SourceMonitor leak. It turns out that we may not need the SourceMonitor as it's
function is provided by the JSP editor. I will be checking in this change today after some more testing.

After that there are still some more leaks:

1. At least on linux there is one leak through the XInputMethod
2. LanguageEditor kit

I will be filing bugs on that separately and make this bug depedent on them.

To the RaveRenderedDocument:
I looked into it, and found only the mapping between source and rendered elements(nodes) and vice versa.
I think the problem is that the source and rendered document are held, not the links between them.

Anyway, first wait for the removal of the SourceMonitor, then we'll see what has left.

Did you mean to change the target milestone from 6.1 to 5.5? I think you accidentally changed that. I'll just mention it
and let you set it to what you need ;-).

Removed the unneeded SourceMonitor. The syntax checking of JSP file is now provided by the JSP editor.

Checking in src/org/netbeans/modules/visualweb/insync/InSyncServiceProvider.java;
/cvs/visualweb/insync/src/org/netbeans/modules/visualweb/insync/InSyncServiceProvider.java,v  <-- 
InSyncServiceProvider.java
new revision: 1.7; previous revision: 1.6
done
Removing src/org/netbeans/modules/visualweb/insync/jsf/SourceMonitor.java;
/cvs/visualweb/insync/src/org/netbeans/modules/visualweb/insync/jsf/SourceMonitor.java,v  <--  SourceMonitor.java
new revision: delete; previous revision: 1.4
done

Back to Peter for further leak detection.

There seems to be the  issue # 105080 for leak through the editorRegistryListener field of JavaSource already. I am
making this dependent of that. I will also add my scenario to that bug. That bug is marked as a duplicate of another bug
105079.

I checked that now, and it seems it is OK in the designer and insync. But there are still other leaks.

This scenario:

1) Open VW project
2) Open Page1
3) Switch to java tab
4) Switch to Jsp tab
5) Switch back to designer tab,
6) Close the Page1
7) Repeat the steps 2) to 6)

There remains one page held by JavaSoure.editorRegistryListener.
So I am passing this to the JavaSource.

There already is an issue that covers the java/source (and editor/hints part) - issue #100753, so I do not see reason
for this issue to be on java/source. If all currently known problems are already filled as different issues, I would
suggest to close this issue. Please note that holding one (last, closed) editor is certainly not a P1 issue:
1. it is not a cumulative leak (as I understand the original report in this issue spoke about a cumulative leak)
2. the user usually has an editor opened, in which case the currently visible editor is referenced, and so no leak occurs

This is not to say that holding the last, closed editor is correct - it is not correct, but certainly not P1 issue.

I'm also seeing a memory leak in JavaSource.binding that holds onto 3 JsfTopComponent objects (through a listener list)
after opening/closing 20+ pages.

I still noticed that all the RaveRenderedDocument objects are still in memory after closing the project, so there is a
cumulative leak somewhere.  There seems to be references between RaveRenderedDocument objects through the userData as
well.  I can provide the heap dump if needed.

I used the VehicleIncidentReport sample application to do the measurements.  There is a still a cumulative leak when
opening/closing pages from that project.

Quy, please, have a look who holds the RaveRenderedDocument's, or provide the heap dump. Thanks.

OK, I tried to simulate what Quy said and found that the RaveRenderedDocument leaks 1 per 1 FacesModel. Since the
FacesModel is still kept, that is the problem. The link is FacesModel holds MarkupUnit holds RaveRenderedDocument.

Passing to insync.

Well after talking to Peter it seems he was not closing the Project whereas Quy's scenario was after closing the
project. In case of Peter's scenario the FacesModel is not leaking as the FacesModel lives as long as the project is open.
Peter is right that the original scenario of this issue does not involve closing the project. May be Quy should file a
separate issue for that.

We think that the page open/close related leaks have been taken care of in VW code. Marking as fixed. Quy will file a
spearate issue for Project close scenario.

Also see the bugs this issue depends on.

There still appears to be a JsfTopComponent memory leak.  Attaching a screenshot from the NB heap dump viewer.

Created attachment 54213 [details]
screenshot of memory leak

Quy, please, provide scenario how you got the leak.
The screenshot doesn't prove anything, it doesn't show who holds the component (i.e. it doesn't show the path to GC root).

I sent you an email yesterday with the project and instructions on how to set it up.  It should be fairly easy to
reproduce the issue from there (opening and closing the start page repeatedly).

Well, but that one is about a bit something else, we figured out that it was problem of firing the desingContextChanged
events from insync while there were no changes made.
Passing there.

OK, the last comment belongs to issue #122931.

This one is about DesignerCaret listener (its Handler inner class) not getting removed, it seems there is missed
deinstall call).
Anyway it seems it will be better to use weak listener instead.

Fixing memory leak caused by listener in DesignerCaret, now using weak listener.

Checking in visualweb/designer/src/org/netbeans/modules/visualweb/text/DesignerCaret.java;
/cvs/visualweb/designer/src/org/netbeans/modules/visualweb/text/DesignerCaret.java,v  <--  DesignerCaret.java
new revision: 1.25; previous revision: 1.24
done

I downloaded 6.1 nightly build and gave it a try. There's a huge improvement, but there MIGHT still be a small (5-10mb)
leak somewhere:

- Opened NetBeans 6.1 200801290002
- Clicked memory meter. GC made memory go down to 29.1 MB
- Opened one Visual Web page. Memory went up to 81.7
- Clicked memory meter, GC made memory go down to 42 MB
- Opened 20 visual web pages, 7 page fragments and 1 html file. Memory meter at 113 MB (used to be close to 280 MB
before these fixes).
- Clicked memory meter, GC made memory go down to 74 mb
- Closed all documents, then clicked memory meter to GC again. Down to 59.1 mb
- Opened all pages and fragments again, then clicked memory meter to GC again. 76.8mb
- Closed all documents, then clicked memory meter to GC again. Down to 65.7 mb
- It appears to leak about 5-10 MB each time I open then close all pages.

Heap dump can be found at http://www.ijws.com/~ryan/netbeans/heap61.bin

The remaining memory leaks are being tracked in Issue 123530.

Marking these fixes verified for the next patch release. Thank you, Ryan.

The fixes have been ported into the release601_fixes branch.

Checking in WebForm.java;
/cvs/visualweb/designer/src/org/netbeans/modules/visualweb/designer/Attic/WebForm.java,v  <--  WebForm.java
new revision: 1.176.8.1; previous revision: 1.176
done
Checking in DescriptionGroupImpl.java;
/cvs/web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/Attic/DescriptionGroupImpl.java,v  <-- 
DescriptionGroupImpl.java
new revision: 1.3.8.1; previous revision: 1.3
done
Checking in IconImpl.java;
/cvs/web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/Attic/IconImpl.java,v  <--  IconImpl.java
new revision: 1.2.8.1; previous revision: 1.2
done
Checking in LocaleConfigImpl.java;
/cvs/web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/Attic/LocaleConfigImpl.java,v  <--  LocaleConfigImpl.java
new revision: 1.2.8.1; previous revision: 1.2
done
Checking in ManagedBeanImpl.java;
/cvs/web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/Attic/ManagedBeanImpl.java,v  <--  ManagedBeanImpl.javanew
revision: 1.7.8.1; previous revision: 1.7
done
Checking in NavigationCaseImpl.java;
/cvs/web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/Attic/NavigationCaseImpl.java,v  <--  NavigationCaseImpl.java
new revision: 1.6.8.1; previous revision: 1.6
done
Checking in NavigationRuleImpl.java;
/cvs/web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/Attic/NavigationRuleImpl.java,v  <--  NavigationRuleImpl.java
new revision: 1.5.8.1; previous revision: 1.5
done
Checking in ResourceBundleImpl.java;
/cvs/web/jsf/src/org/netbeans/modules/web/jsf/impl/facesmodel/Attic/ResourceBundleImpl.java,v  <--  ResourceBundleImpl.java
new revision: 1.2.8.1; previous revision: 1.2
done
Checking in OutlinePanelProvider.java;
/cvs/visualweb/outline/src/org/netbeans/modules/visualweb/outline/Attic/OutlinePanelProvider.java,v  <-- 
OutlinePanelProvider.java
new revision: 1.7.8.1; previous revision: 1.7
done
Checking in InSyncServiceProvider.java;
/cvs/visualweb/insync/src/org/netbeans/modules/visualweb/insync/Attic/InSyncServiceProvider.java,v  <-- 
InSyncServiceProvider.java
new revision: 1.6.10.1; previous revision: 1.6
done
Removing SourceMonitor.java;
/cvs/visualweb/insync/src/org/netbeans/modules/visualweb/insync/jsf/Attic/SourceMonitor.java,v  <--  SourceMonitor.java
new revision: delete; previous revision: 1.4.10
done
Checking in DesignerCaret.java;
/cvs/visualweb/designer/src/org/netbeans/modules/visualweb/text/Attic/DesignerCaret.java,v  <--  DesignerCaret.java
new revision: 1.24.12.1; previous revision: 1.24
done