Flyer53 / jsPanel4

A JavaScript library to create highly configurable floating panels, modals, tooltips, hints/notifiers/alerts or contextmenus for use in backend solutions and other web applications.
https://jspanel.de/
Other
313 stars 57 forks source link

Memory Leaks after closing Panels #99

Closed lennart-bader closed 4 years ago

lennart-bader commented 4 years ago

Hi, I noticed a performance drop when opening and closing jsPanels several times due to detached Divs.

After closing a panel, Chrome Developer Tools show several Detached HTMLDivElements. So after opening and closing 10 panels, I get about 150 detached Divs (with empty panels).

This results in increasing detached EventListeners as well when having elements with said listeners inside the panel - however, this might just be a bug of the Chrome Developer Tools, which seem to have a bug with garbage collection during performance recordings.

Using panels with actual content increases the memory leak - in my application where I experienced the issue first, the number of Node explodes rapidly and causes the performance drop.

Most of the DivElements have context in jspanel.js line 1997 (when I'm using the DevTools correctly...)

I made a small example here:

https://jsfiddle.net/kryptur/gxst7cud/5/

Taking a Heap Snapshot before and after pressing the Button several times and closing the panels again leads to the described phenomenon.

This is the DevTool Memory Summary:
image

Any help would be appreciated.

Flyer53 commented 4 years ago

@kryptur I don't think it's a bug in the Chrome dev tools. The same can be observed in FF dev tools. As far as I understand it it's a particularity of the DOM and JavaScript implementations in a browser - see https://stackoverflow.com/questions/3785258/how-to-remove-dom-elements-without-memory-leaks But so far I did not observe any noticeable performance issues.

It might well be possible to reduce memory leakage with proper code, but to be frank ... memory leakage and similar stuff are topics I never dealt with as an amateur programmer.

Bottom line: I don't think I can do something about that any time soon but would appreciate any tip.

PS: Since you seem to be in Germany as well you can always email me in German - info@jspanel.de

lennart-bader commented 4 years ago

Thanks for having a look into this - in case anyone else comes accross this issue, I would suggest keeping the discussion here if this is ok for you.

According to https://stackoverflow.com/questions/8138742/do-any-web-browsers-garbage-collect-removed-dom-elements-as-opposed-to-javascr?noredirect=1&lq=1, Chrome actually DOES garbage collection on unused Nodes. This, however, only happens on unreferenced nodes - otherwise the node might not be garbage.
I forced garbage collection in the JSFiddle snippet and it did nothing.

I looked into your Code and my best suggestion would be that you still reference the Div of the panel (and thus, the whole subtree with all attached listeners) around line 1136 as suggested by DevTools. There you attach listeners to the document.

dragit: function dragit(elmt) {
  // ...
  jsPanel.pointerup.forEach(function (item) {
      document.addEventListener(item, function (e) {
             jsPanel.pointermove.forEach(function (e) {
                      document.removeEventListener(e, dragElmt);
             });
             // ...
            if (dragstarted) {
                 elmt.style.opacity = 1; // <--- THIS might be the issue
            }
      });
  });
}

The problem might be that inside the listener attached to the document, the elmt defined in the outer function (dragit) is used. Thus, this can never be garbage collected.
Also the item variable could be a candidate for this. But I'm not sure what this variable actually contains.

Maybe take a look here: https://stackoverflow.com/a/16936890/6237870 (Answer part c).

So obtaining elmt inside the listener from e.target or something might solve the issue.

I don't have much experience with memory leaks in Javascript, neither (just dove into this topic when I experienced the described issues).

Regarding the Performance Issues:
I was using your jsPanels to display configuration options, i.e., there were dynamically created forms attached to the panel. There were several listeners attached to the inputs (think of 30 inputs per panel).
As a result, opening and closing the panel several times (e.g. 10 times) created thousands of listeners (also by third party libraries) and thousands of detached dom nodes. The background animation (drawn to a canvas) dropped from 60FPS (target rate) to 20 FPS. When I did not open any panel, the FPS stayed at 60.

Since I might not use the panels in a common way, I don't think it's actually worth wasting much time on this issue.
I will try the jQuery-UI dialogs an test if the problem occurs then, too (then it might actually be my fault or the fault of another library).

Small Update I tested it with jQuery UI Dialogs - the amount of listeners went down drastically after closing the Dialog.
The increase and decrease of the yellow line (listeners) happens directly when opening and closing the Dialog. The blue drop at the end is the forced garbage collection.
I don't know if the amound of nodes will get garbage collected at some point, but the performance drop is gone.

image

Flyer53 commented 4 years ago

@kryptur Thanks for your extra tests and sharing the results. Well, at the time being I don't have a lot of time to work on jsPanel, altough there are a few things I'd like to get done. There might be a v4.8.0 release later this month. But that has only a few minor updates/changes. Until something substantial happens you'll have to wait til next year I guess. So be patient please ;)

Regards, Stefan

Flyer53 commented 4 years ago

@kryptur I just released v4.9.0. There won't be any change concerning this issue though, and I don't see anything I could do about it for now but I'm open for ideas if you know something ... For now I close the issue, but keep it in mind.

Regards, Stefan