Mapael: leaking problems

[Indigo744]

After reading #43, something said by RASMiranda worried me:

you could refresh the entire map by emptying the appropriate DIV container and calling again $.mapael()with the new options

I tried exactly this with ajax, but don't know if it's because of my map, but after the first 5 updates page starts getting slow and the browser not responding...

Holly cow, that is typically a leaking problem!

New map test

I made a script to test it:
Every seconds:

1. Empty mapcontainer
2. Replace mapcontainer content with initial HTML (.map and .myLegend divs)
3. Create Mapael on it
   Here is a screenshot of Chrome debugger for 10 iterations:
   

We can see the 10 occurences through the number of listeners (in orange). They represents the number of events attached to elements on the page. They drop after calling $.empty() because jQuery take care of removing all event handlers.
Now let's check the number of nodes (in green). This is the number of DOM nodes retained in memory. And as you can see, it is always increasing after each call to Mapael! And even after forcing the GC => note the JS heap (blue line) drop at the end, but not the number of nodes.
Here is a leak!
This is why after a while the page starts getting slow and not responsive.

Updated links test

I made a script to test the update of links (links creation and removal):

1. Create Mapael on mapcontainer with some initial plots/links
   Every seconds:
2. Trigger update: delete all plots/links (with deletedPlots = "all" and such)
3. 500ms timeout: Trigger update: add some new plots/links
   Here is what we got:
   
   The number of nodes and listeners are always increasing, and forcing a GC (blue line drop at the end) doesn't reduce this number.
   This is bad. With more plots/links, this will lead to a page more and more slow.

Updated legends test

I made a script to test the update of legends (legend replacement, implemented through #87):

1. Create Mapael on mapcontainer with an initial legend
   Every seconds:
2. Trigger update: set a new legend
   We have this:
   
   Wow! At last, something is not leaking.
   The number of nodes are increasing, but when forcing GC (at the end), it will drop to the initial one.
   This is good. Updating the legend will not lead to a leaking problem.

Now, what to do?

I have some ideas to correct the leaking. I will submit some PR and we will be able to discuss it.
Also, I want to submit the code for the test scripts I used. They are modified version of existing example, with loops and such. Do you think I can create a test directory and put these files in it?

[neveldo]

Wow, thank you for all the tests you have run in order to highlight leak problems regarding the map update and deletion. I should have focused myself more on checking leaks ...

I'm glad you have some ideas for fixing the leaks. Regarding the tests, if your tests aim to be permanent, you can add a test directory of course. I think this kind of directory should contains only unit tests or at least permanent test script. If your tests aims to be temporary in order to help fixing the leaking problem, maybe I can create a test branch from the master. The tests would be added only to the test branch, and the test branch would be synchronized with the master regularly to check the fixes. What do you think about it ?

[Indigo744]

I think a test-leak branch would be better. These are not aimed to be run each time.
If you create this branch, can I push PR to it?

[neveldo]

Yes, it works as the master branch so you can push PRs to it. I have created the branch : https://github.com/neveldo/jQuery-Mapael/tree/test-leak .

In order to switch to this branch, you just have to execute : git fetch && git checkout test-leak. Then, you can work on this branch and push new PR as usual. If you want to go back to the master branch, you can execute : git checkout master.

In order to synchronize the test-leak branch with the (commited) modifications made on the master branch, you can execute : git merge master from the test-leak branch.

[Indigo744]

Great! I'll submit a PR soon ;-)

[Indigo744]

This fix correct the second leaking.
Before:

After:

As you can see, the number of listeners is not always increasing. No leaking :-)

[Indigo744]

Uh? No, you should not close this issue. Only a specific part if corrected...!

[neveldo]

Oops, sorry, false manipulation ! Thank you for your first fix, I have merged your PR.

[Indigo744]

No worries :) always glad to help !

[Indigo744]

After last PR, the leaking problem with new map is corrected:

Before

After

As you can see, the number of nodes are not longer always increasing. There is no memory leaks anymore! Yiaie!