Repeatedly calling c3.generate leaks memory in 0.4.9

[ergo]

Hello,

I got reports that pages that constantly regenerate our chart objects use up too much memory and kill browser tabs:

http://plnkr.co/edit/l0JOvGBWRDEv0b3fwywr?p=preview - here is an example with 0.4.9 version

http://plnkr.co/edit/wEQiMgBeBzVAwYTdijKD?p=preview - here is an example with 0.4.8, it seems to leak way less and is way more stable according to my tests.

[aendra-rininsland]

@ergo Interesting; is there any reason you're calling destroy and then loading more data instead of just using api.load? I agree with you though, it seems there's an issue — I changed to a different tab for a minute and the Plunker tab crashed.

We've noticed people are having issues with D3 latest; does the situation improve if you downgrade D3 to, say, 3.4.3 (which is what c3js.org is currently using)?

[ergo]

Yes, the reason is c3 doesn't support setting some options on load() - like changing chart types of existing series or setting new labels, or setting x tick values - this only works when generate() is called. So my workaround is to just generate whole chart - not ideal but seems the only option right now when I will have generative charts and the values might change on update.

And yes I've tried downgrading to 3.5.0 and 3.4.3 - you can fork and edit the plunkers the 0.4.9 one will quickly reach 1.5gb mem regardless of d3 version used :(

[aendra-rininsland]

@ergo Ah, I see — seemed like the data remained sort of constant, which was why I was wondering about load() (FWIW, I do the exact same thing with times/axisJS whenever the user changes chart types, would be really nice to be able to modify charts without having to call c3.generate() again. Might be worth creating a separate issue about improving load()'s functionality).

But yeah, tab just crashed again. Yikes, that's definitely an issue. Marking as bug.

[aendra-rininsland]

There is definitely something leaking memory. I just tried to do a screencast using axisJS and it crashed the browser tab after about 2 minutes on my recent-model Macbook Pro. I'm going to investigate this today, this bug seems pretty dire...

Edit: Worth noting this might be specific to stacked charts — the chart in particular I was making was stacked and your demo is stacked.

Edit2: Nope, it appears even basic line charts are also susceptible to this. I've been watching Chrome's task manager go from 400mb (?!) memory for my ultra-stripped down C3 example to now over a gig (~5 minutes). My feeling is it's something related to garbage collection — it hovered at a pretty stable point for awhile and then memory use started increasing dramatically. It's definitely tied to calling generate repeatedly, though.

[aendra-rininsland]

I'll update this comment as I find more. Caveat: I'm incredibly novice with Chrome's Timeline and Profiles tools.

In the meantime:

• Plunker
• 10 minute heap snapshot here (~230mb)
• Test code:

  var generateData = function() {
    var data = [];
    for (var i = 0; i < 5; i++) {
      var datum = [];
      datum.push('dataset ' + i);
      for (var j = 0; j < 20; j++) {
        datum.push(Math.random() * 1000);
      }
      data.push(datum);
    }

    return data;
  };

  var int;

  var toggleTest = function() {
    if (!int) {
      int = setInterval(function(){
        c3.generate({
          data: {
            columns: generateData()
          }
        });
      }, 3000);
    } else {
      clearInterval(int);
    }
  };

• Calling .unload() before c3.generate() doesn't seem to improve the situation (Plunker). Here's a timeline where I called unload() before each generate(). Heap size, listener count and node count still keep increasing.
• Here's a 10 minute snapshot of the exact same code, but calling unload before c3.generate. Comparing them, the retained size for ChartInternal with unload is 50% smaller, but it still seems like memory's leaking somewhere.
• Using Firefox's dev tools, it seems like c3_chart_internal_fn.getMaxTickWidth has a really high self cost:
• Running that same test with axes off (and no unload), it seems like things improve quite a lot — nothing is anywhere near 20% self cost. Something D3 is doing is up near 14% (I can't tell which, I foolishly used the minified version of D3 because Bostock uses Unicode symbols in D3's code which totally borks python -m SimpleHTTPServer), but it seems like the call stack in general is might smaller.
• That said, running the same 10-minute test in Chrome without any axes still eats up a craptonne of memory. So it's probably not that — it's probably showing up in Firefox because D3's each() method is called frequently, and I can't imagine that's very fast.
• Looking at Chrome's profiling, it seems there are a LOT of detached DOM nodes that aren't getting GC'd. If you look at the object properties, SVGCircleElement > mainCircle > $$ > resizeFunctions() comes up a LOT.

I can't spend any more time today looking at this, but my feeling is that resizeFunctions() isn't nullifying references after a generate() and thus GC isn't doing much.

But then again — I know an infinitesimally small amount about JavaScript performance and...

[aendra-rininsland]

Actually, see this comment on #172. generateResize() comes up there too.

Here's what I think is happening. generateResize() is attached to the window object, and all the resize functions keep getting pushed into the resizeFunctions array regardless of anything that happens with generate(). As a result, these functions hold references to expired SVG DOM elements, and these are never GC'd.

Edit: Yeah, that's totally what's happening. Add a breakpoint to ln. 896 and add resizeFunctions as a Watch Expression. Let the test run for awhile (i.e., let generate be called a bunch of times) and resize the browser window. Note the length of resizeFunctions. Resume execution, wait awhile, resize again. The length of resizeFunctions should be consistent, but because generate doesn't empty resizeFunctions, none of these are ever released.

[masayuki0812]

Thank you for the investigation. I also looked into this and noticed this seems to happen when the tab where the code running is not visible (Strictly speaking, still leaking, but it looks quite smaller). Additionally, the PR @aendrew created #938 seems to fix this issue, so the transitions when the tab is not visible would be the cause. And there are still some transitions even if we set transition.duration = 0 or apply #938 , so I'll try to disable all of transitions.

[aendra-rininsland]

@masayuki0812 Worth noting all the performance monitoring stuff I did was with the C3 chart tab active. I'm pretty sure it's a detached DOM nodes issue and not a requestAnimationFrame issue. AFAIK, the only way to fix this is to improve garbage collection, which is to empty resizeFunctions on generate() instead of just appending new functions to it.

[snkashis]

+1. Yes, @masayuki0812 I also noticed the transition.duration = 0 disabling doesn't seem to work in all cases. Would another idea be some sort of global c3.TRANSITIONS_ENABLED switch so it doesn't need to be specified for each visualization? Able to be overridden though of course if needed...

[masayuki0812]

@aendrew Yeah, I'm not saying #938 fixes the issue completely, it just makes the speed of leak slower as @ergo wrote in https://github.com/masayuki0812/c3/issues/926#issue-54998863 . Did you confirm it fixes this issue and can you create PR about this?

@snkashis can you create new issue? I expect it would be something like to disable all transitions.

[ergo]

@masayuki0812 - Any news on this? I would love to update to latest stable but unfortunately have to wait till the leak is gone :)

[JamieMason]

FYI we've just upgraded to 0.4.11-rc1 and have found the memory leak to be much less severe now. In our case we're calling c3.generate(...) once and then .load(...) on the returned instance — but c3 was leaking memory in our case as well.

[aendra-rininsland]

@JamieMason Thanks for the update! The improved performance when using .load is due to #1113. Still need to squash this "calling generate repeated" bug though — so much of the C3 API is only exposed via .generate!

[chrismingay]

Really appreciate your efforts with this. I just stumbled upon the memory leak updated c3js-directive.js so that in loadChartData() generate is only called once and after that the load() function is called.

if(!$scope.generated){
    console.log("This is the first load");
    $scope.chart = c3.generate($scope.config);    
    $scope.generated = true;
}else{
   $scope.chart.load($scope.config.data); 
}

I know this doesn't help for those who need to repeatedly generate() but it helped with my chart data stream needs!

[bnest]

Any further news on this? I created a single page app with C3 charts and after all I implemented an auto-kiosk-slide mode, where all my diagrams were generated in a timer. .... the page crashes after about 2h of working. the javascript heap-size is full and I can confirm that generating new OR not properly deleting OLD diagrams causes a HUGE memory leak.

[ahalota]

I'm having the same issues now when using c3.generate() to make a few charts, removing the old ones (by removing DOM elements), and then creating new ones, it gets slower and slower.

@bwestpha How do I properly delete old diagrams?

[ahalota]

I see now the method is called "destroy", I had been looking for remove or delete, I'll give that a try.

[kelvinau]

I found a temporary solution that can fix this memory issue. Just use the fallback code

        // fallback to this, if this is a very old browser
        var wrapper = window.onresize;
        if (!wrapper) {
            // create a wrapper that will call all charts
            wrapper = $$.generateResize();
        } else if (!wrapper.add || !wrapper.remove) {
            // there is already a handler registered, make sure we call it too
            wrapper = $$.generateResize();
            wrapper.add(window.onresize);
        }
        // add this graph to the wrapper, we will be removed if the user calls destroy
        wrapper.add($$.resizeFunction);
        window.onresize = wrapper;

instead at https://github.com/c3js/c3/blob/fc6905129ebedde0968eb63097fc08531a843ee7/c3.js#L1989, and also use

        var wrapper = window.onresize;
        // check if no one else removed our wrapper and remove our resizeFunction from it
        if (wrapper && wrapper.add && wrapper.remove) {
            wrapper.remove($$.resizeFunction);
        }

at https://github.com/c3js/c3/blob/fc6905129ebedde0968eb63097fc08531a843ee7/c3.js#L3479

My guess is window.removeEventListener is unable to remove the handler $$.resizeFunction because $$.generateResize returns a new instance of callResizeFunctions every time, and so addEventListener is not keeping the reference to the original callResizeFunctions

[kelvinau]

Update: the above suggestion will remove the feature of onresize & resized, so it's not a correct fix.

destroy() can actually remove the event listeners, so that is not the problem. My problem was because of Angular's issue. It tries to destroy the element before rendering it when I navigate to the page very fast a few times, and so the events binded to window are left and not destroyed. I just check if the element exists before rendering and it solves the memory issue.

But still, I don't understand the reason that if the element doesn't exist, c3 still does everything and bind the events. Why doesn't it just throw an error instead?

[nazihahmed]

hi, I have fixed all of the issues regarding the memory leak https://github.com/c3js/c3/pull/2272 I use c3 heavily within an angular application until recently our clients started reporting browser crashing issues, I had 2 options either fix it or use a different library, https://github.com/Aloomaio/c3

[nazihahmed]

https://github.com/c3js/c3/pull/2273

[nazihahmed]

note to make sure the memory leak doesn't happen you must call destroy