ggeorg / gwt-mosaic

Automatically exported from code.google.com/p/gwt-mosaic
1 stars 0 forks source link

WindowPanel leaks #51

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
i'm working on a long-running app that uses WindowPanel extensively and i
noticed that memory usage is growing without bound.

What steps will reproduce the problem?
1. create a series of WindowPanel instances
2. watch memory consumption

What is the expected output? What do you see instead?

there are no references to these instances and they are not attached to the
DOM but they still hold memory. i would expect them to get collected and
memory usage to be stable.

What version of the product are you using? On what operating system?

GWT 1.6.4

gwt-beans-binding-0.2.3.jar
gwt-dnd-2.6.2.jar
gwt-incubator-trunk-r1650.jar
gwt-log-2.6.0.jar
gwt-mosaic-0.2.0.jar
gwtx-1.5-20081912.jar

Original issue reported on code.google.com by jeremiah...@gmail.com on 19 May 2009 at 2:43

GoogleCodeExporter commented 9 years ago
import com.google.gwt.core.client.EntryPoint;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.user.client.ui.DialogBox;
import com.google.gwt.user.client.ui.VerticalPanel;
import com.google.gwt.user.client.ui.RootPanel;

import org.gwt.mosaic.ui.client.WindowPanel;

public class WindowPanelLeak implements EntryPoint
{
    public void onModuleLoad()
    {
        Button windowPanelButton = new Button("Make 100 WindowPanel", new
ClickHandler() {
            public void onClick(ClickEvent event) {
                for (int i = 0; i < 100; i++)
                    new WindowPanel();
            }       
        }); 
        Button dialogBoxButton = new Button("Make 100 DialogBox", new ClickHandler() {
            public void onClick(ClickEvent event) {
                for (int i = 0; i < 100; i++)
                    new DialogBox();
            }       
        }); 
        VerticalPanel vpanel = new VerticalPanel();
        vpanel.add(windowPanelButton);
        vpanel.add(dialogBoxButton);
        RootPanel.get().add(vpanel);
    }   
}   

Original comment by jeremiah...@gmail.com on 19 May 2009 at 2:44

GoogleCodeExporter commented 9 years ago
the app was compiled on mac. i tested in ie7 and chrome 1.0.154.65 on windows, 
and
firefox 3.0.10 on mac.

Original comment by jeremiah...@gmail.com on 19 May 2009 at 2:46

GoogleCodeExporter commented 9 years ago

Original comment by georgopo...@gmail.com on 19 May 2009 at 8:38

GoogleCodeExporter commented 9 years ago
hi,

some progress:

http://code.google.com/p/gwt-mosaic/source/detail?r=870
http://code.google.com/p/gwt-dnd/issues/detail?id=70&colspec=ID%20Type%20Status%
20Priority%20Milestone%20Stars%20Summary

Kind Regards,
George.

Original comment by georgopo...@gmail.com on 28 May 2009 at 4:19

GoogleCodeExporter commented 9 years ago
hi george,

i'd like to test but the fix appears to depend on a change in gwt-dnd:
[ERROR] Line 261: The field AbstractDragController.mouseDragHandler is not 
visible

i'm having trouble building gwt-dnd. for gwt-dnd-2.6.2 should i be building 
trunk or
the GWT1.6 branch? there's no build instructions for dnd and the build.xml is a 
bit
weird.

could you advise how to build or provide an updated jar?

thanks very much.

Original comment by jeremiah...@gmail.com on 28 May 2009 at 11:05

GoogleCodeExporter commented 9 years ago
Hi,

just copy those two files into your project (but keep the package name):

http://code.google.com/p/gwt-mosaic/source/browse/branches/GWT-1.6/src/com/allen
_sauer/gwt/dnd/client/

I hope I get soon news from gwt-dnd author, if not I am going to build a custom
gwt-dnd.jar for gwt-mosaic-0.2.1 this week end.

Thanks,
George.

Original comment by georgopo...@gmail.com on 28 May 2009 at 11:19

GoogleCodeExporter commented 9 years ago
hmm. i have those files but it seems like the ones from gwt-dnd.jar are being 
used
instead of the modified ones. i'm not sure what determines which gets used. do 
you
know how to fix?

good to know you are pursuing the gwt-dnd update with fred sauer.

Original comment by jeremiah...@gmail.com on 28 May 2009 at 11:38

GoogleCodeExporter commented 9 years ago
Hi,

I patched gwt-dnd-2.6.2 so that there is no problem is people use SVN version of
gwt-mosaic

http://code.google.com/p/gwt-mosaic/source/detail?r=875

Kind Regards,
George.

Original comment by georgopo...@gmail.com on 29 May 2009 at 5:54

GoogleCodeExporter commented 9 years ago
thanks for the dnd build. i am able to test now.

do you intend to support re-showing a WindowPanel after it has been hidden? my 
app
has a WindowPanel with heavy contents that is kept around to show() and hide() 
as
needed. right now show() is throwing if it is called after hide():

java.lang.NullPointerException
  at org.gwt.mosaic.ui.client.WindowPanel.show(WindowPanel.java:1566)
  at
org.gwt.mosaic.ui.client.DecoratedLayoutPopupPanel.pack(DecoratedLayoutPopupPane
l.java:149)
  at org.gwt.mosaic.ui.client.WindowPanel.showModal(WindowPanel.java:1605)
  at org.gwt.mosaic.ui.client.WindowPanel.showModal(WindowPanel.java:1584)

can you move the windowController setup from the constructor to show() so it is
symmetrical with the tear-down in hide()?

Original comment by jeremiah...@gmail.com on 29 May 2009 at 12:56

GoogleCodeExporter commented 9 years ago
the minimize button in this demo 

http://69.20.122.77/gwt-mosaic/Showcase.html#CwWindowPanel

I think is what you need, it keeps the state in case you use an IFRAME (see 
"Sized"
button, click on a link move the WindowPanel and click minimize, after that 
click
again the "Sized" button).

the issue with hide()/show() I think is introduced after my last commit because 
it
does some clenups in the hide() method. I have to think about that because some
intialization code must go into the onLoad() method and not like it is now in 
the
constructor.

Original comment by georgopo...@gmail.com on 29 May 2009 at 3:29

GoogleCodeExporter commented 9 years ago
hmm.. the problem with using minimize as a work-around for show/hide is that 
minimize
is not available for modal popups.

when WindowPanel no longer leaks i suppose i can just hang onto the content 
panel and
create a new WindowPanel instance for every show().

Original comment by jeremiah...@gmail.com on 29 May 2009 at 4:39

GoogleCodeExporter commented 9 years ago
hi,

the code now in show()/hide() is symmetrical.

Kind Regards,
George.

Original comment by georgopo...@gmail.com on 2 Jun 2009 at 6:16

GoogleCodeExporter commented 9 years ago
Hi,

now this code works OK. I checked with FF3 on Linux with 100 button clicks, the
garbage collector seems to work. But the issue is not fixed yet. show()/hide() 
still
leaks.

  public void onModuleLoad() {
    Button windowPanelButton = new Button("Make 100 WindowPanel",
        new ClickHandler() {
          public void onClick(ClickEvent event) {
            for (int i = 0; i < 100; i++) {
              WindowPanel w = new WindowPanel();
              //w.show();
              //w.hide();
              System.out.println(count);
              count++;
            }
          }
        });
    Button dialogBoxButton = new Button("Make 100 DialogBox",
        new ClickHandler() {
          public void onClick(ClickEvent event) {
            for (int i = 0; i < 100; i++) {
              DialogBox d = new DialogBox();
              //d.show();
              //d.hide();
              System.out.println(count);
              count++;
            }
          }
        });
    VerticalPanel vpanel = new VerticalPanel();
    vpanel.add(windowPanelButton);
    vpanel.add(dialogBoxButton);
    RootPanel.get().add(vpanel);
  }

/George.

Original comment by georgopo...@gmail.com on 2 Jun 2009 at 4:13

GoogleCodeExporter commented 9 years ago
Hi,

I think FF3/Linux does not leak any more. Try this:

  public void onModuleLoad() {
    Button windowPanelButton = new Button("Make 100 WindowPanel",
        new ClickHandler() {
          public void onClick(ClickEvent event) {
            for (int i = 0; i < 100; i++) {
              final WindowPanel w = new WindowPanel("" + count);
              DeferredCommand.addCommand(new Command() {
                public void execute() {
                  w.pack();
                  new DelayedRunnable(10000) {
                    @Override
                    public void run() {
                      w.hide();
                    }
                  };
                }
              });
              System.out.println(count);
              count++;
            }
          }
        });
    RootPanel.get().add(windowPanelButton);
  }

/George.

Original comment by georgopo...@gmail.com on 2 Jun 2009 at 6:59

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
i've just run my original test case (just creating the widget, not showing it) 
with r889.

safari/chrome show big improvement though still seem to leak a small amount. 
however,
under ie7 WindowPanel is still leaking a lot. every press of the '100 
WindowPanel'
button causes about ~11mb of memory growth.

Original comment by jeremiah...@gmail.com on 2 Jun 2009 at 8:54

GoogleCodeExporter commented 9 years ago
I am going to test with Windows.

Thanks,
George.

Original comment by georgopo...@gmail.com on 2 Jun 2009 at 9:02

GoogleCodeExporter commented 9 years ago
thanks very much.

Original comment by jeremiah...@gmail.com on 2 Jun 2009 at 9:04

GoogleCodeExporter commented 9 years ago
Hi,

good tool to find out memory leaks (at least in hosted mode) is:

http://www.eclipse.org/mat/

The results for:

  public void onModuleLoad() {
    Button windowPanelButton = new Button("Make 100 WindowPanel",
        new ClickHandler() {
          public void onClick(ClickEvent event) {
            for (int i = 0; i < 100; i++) {
              new WindowPanel("" + count);
              System.out.println(count);
              count++;
            }
          }
        });
    RootPanel.get().add(windowPanelButton);
  }

are in the attached screenshot (OS: Linux, generated WindowPanel: 1000).

The reposrt shows that new WindowPanel() does not leak (I have still to run the 
demo
on Windows hosted mode).

A report for hide()/show() will follow.

Kind Regards,
George.

Original comment by georgopo...@gmail.com on 3 Jun 2009 at 12:16

Attachments:

GoogleCodeExporter commented 9 years ago
Hi

for:

  public void onModuleLoad() {
    Button windowPanelButton = new Button("Make 100 WindowPanel",
        new ClickHandler() {
          public void onClick(ClickEvent event) {
            for (int i = 0; i < 100; i++) {
              WindowPanel w = new WindowPanel("" + count);
              w.pack();
              w.hide();
              System.out.println(count);
              count++;
            }
          }
        });
    RootPanel.get().add(windowPanelButton);
  }

see the attached report.

Some WIndowPanel instances are still there and some other objects that build
WindowPanel like ElementDragHandle (each WindowPanel has 8 ElementDragHandles).

I don't understanf why there are only 13 WindowPanel but it seems that 
WindowPanel
leaks somehow in Linux hosted mode (old Gecko).

I will check with windows too.

/George.

Original comment by georgopo...@gmail.com on 3 Jun 2009 at 1:24

Attachments:

GoogleCodeExporter commented 9 years ago
Hi,

I am testing on Windows XP Pro with IE7 with:
-----------------
public class Test implements EntryPoint {
  private int count = 0;

  public void onModuleLoad() {
    Button windowPanelButton = new Button("Make 100 WindowPanel",
        new ClickHandler() {
          public void onClick(ClickEvent event) {
            for (int i   = 0; i < 100; i++) {
              WindowPanel w = new WindowPanel("" + count);
              System.out.println(count);
              count++;
            }
          }
        });
    Button dialogBoxButton = new Button("Make 100 DialogBox",
        new ClickHandler() {
          public void onClick(ClickEvent event) {
            for (int i   = 0; i < 100; i++) {
              DialogBox w = new DialogBox();
              w.setWidget(Caption.IMAGES.windowClose().createImage());
              System.out.println(count);
              count++;
            }
          }
        });
    RootPanel.get().add(windowPanelButton);
    RootPanel.get().add(dialogBoxButton);
  }

}
-----------------
With http://www.eclipse.org/mat/ in hosted mode everything is OK. But, by using 
the
browser (IE7) both WindowPanel and DialogBox seems to leak memory. I tried
WindowPanel without the image and it works without leeking.

I replaced the image with the jimmy.jpg image used in GWT 1.6.4 Showcase:

http://gwt.google.com/samples/Showcase/Showcase.html#CwDialogBox

but still DialogBox leaks memory too.

I will try to find if there is a workaround for this... if not try to use a
WindowPanel pool and reuse closed WindowPanels.

Thanks,
George.

Original comment by georgopo...@gmail.com on 4 Jun 2009 at 1:44

GoogleCodeExporter commented 9 years ago
i also saw the difference between hosted and web mode but i'm not sure what to
attribute it to.

yeah... i am using a pool of WindowPanels now.

thanks very much for the debugging effort.

Original comment by jeremiah...@gmail.com on 4 Jun 2009 at 1:53

GoogleCodeExporter commented 9 years ago
Hi, 

is it possible for you to test it with IE8?

I check and it works without leaking.

Thanks,
George.

Original comment by georgopo...@gmail.com on 4 Jun 2009 at 2:16

GoogleCodeExporter commented 9 years ago
Hi,

this may be a good workaround for IE7 (see lines: 698 & 730):

http://code.google.com/p/gwt-mosaic/source/detail?r=893

It seems that AbstractImagePrototype.createImage() creates leaks in IE7.

Let me know if that works.

Please also read: http://www.pathf.com/blogs/2008/01/gwt-imageprefet/

/George.

Original comment by georgopo...@gmail.com on 4 Jun 2009 at 3:18

GoogleCodeExporter commented 9 years ago
http://code.google.com/p/google-web-toolkit/issues/detail?can=2&q=&colspec=ID%20
Type%20Status%20Priority%20Milestone%20Owner%20Summary&sort=&id=3588

Original comment by georgopo...@gmail.com on 4 Jun 2009 at 6:15

GoogleCodeExporter commented 9 years ago
it looks like the AbstractImagePrototype.createImage() workaround made a big 
impact.

i've just tested with r896 on ie7 and memory usage is hugely improved. i tried 
both
my original test and a test with create, show, hide. in both cases memory stays 
in check.

Original comment by jeremiah...@gmail.com on 9 Jun 2009 at 5:06

GoogleCodeExporter commented 9 years ago
looks good in ie8 as well.

Original comment by jeremiah...@gmail.com on 9 Jun 2009 at 6:46

GoogleCodeExporter commented 9 years ago
Hi,

yes it looks good.

this week I don't have too much time, but I am going to continue checking for 
memory
leaks.

Thanks,
George.

Original comment by georgopo...@gmail.com on 9 Jun 2009 at 7:06