C3: Memory leak in config.onrendered

[yunyu]

Related issues: https://github.com/c3js/c3/issues/1183 https://github.com/c3js/c3/issues/1097

The fundamental issue is that the config.onrendered queue never gets cleaned up or flushed (ever) when the tab is inactive.

It stores a bunch of functions which isn't exactly memory efficient, resulting in even switching from the tab for 30 seconds causing a massive garbage collection hang.

(It also causes the transitions to be applied in a buggy fashion, e.g. I have the length option set in a flow() which is not applied properly on resume, none of the points are removed from the graph like when the tab is active):

This single issue makes c3 unusable for a lot of situations. It seems like removing the queueing altogether (or at least periodically flushing it), or at the very minimum making sure that only one call is queued for each chart would be for the best. This is a great library with lots of features, and I really hope that this 2 year old issue can be fixed.

[aendrew]

@yunyun I quite agree, I'd love to get that issue resolved, it's plagued C3 for years. Thanks for the thorough analysis, your grasp of memory profiling seems a bit better than mine.

Any chance I could convince you to submit a PR? 😄

[yunyu]

I'm not too familiar with the C3 codebase (just a general glimpse), but I'm going to be working with it for my Google summer of code project so I might be able to help. I think replacing all the internal timer/queue implementations with https://github.com/d3/d3-timer would likely fix this (it periodically does a setTimeout instead of requestAnimationFrame for inactive tabs) as well as many other performance issues with lots of c3 graphs (it's the official implementation, after all).

[aendrew]

@yunyu That sounds excellent — though no idea if D3 v3 has any compatibility issues with d3-timer. Feel free to drop me a line on Gitter if there's anything I can help you with; again, this is something I'd love to get sorted out. 🙌

[yunyu]

@aendrew d3-timer doesn't seem to actually have any reliance on d3 v4 (a standalone version is offered, and an internal version has been shipped with d3 forever), but the difficult part would be to temporarily disable all transition durations when the tab is inactive in order to stay under the Chrome CPU budget. This is done automatically in v4, but it would likely be a somewhat complex affair (from what I've seen with how chart.internal.config.transition_duration is handled and how spread out the uses are, in addition with flow() using a separate transition).

On a hacky note, I've worked around the original issue in my application (http://twitterwall.yunyul.in/admin) by just destroying and recreating all charts on resume.

[yunyu]

Just adding a minimal repro case: https://jsfiddle.net/f11x2012/3/

[yunyu]

I attempted to backport d3-timer to d3 v3.5.17 (while stripping the inactive tab handling in c3), but unfortunately, the periodic-wake behavior was removed in https://github.com/d3/d3-timer/commit/da49992b1f5d91316268fe3039b91746157d53ed due to upstream changes in d3-transition, and there was no noticeable improvement in background behavior compared to current c3 with the original v3.5.17.

Info: https://medium.com/@mbostock/what-makes-software-good-943557f8a488

D3 4.0 fixes this problem by changing the definition of time. Transitions don’t typically need to be synchronized with absolute time; transitions are primarily perceptual aids for tracking objects across views. D3 4.0 therefore runs on perceived time, which only advances when the page is in the foreground. When a tab is backgrounded and returned to the foreground, it simply picks up as if nothing had happened.

Furthermore, I attempted to reimplement the wakeup behavior, but it quickly exceeded Chrome's background tab budget (even with fully disabled transitions) and resulted in minimal change in memory usage. I think this issue isn't really fixable without moving to d3 v4 or monkeypatching in the new transitions/selections modules (which is planned anyway), and the recreate-graph-on-resume-with-disabled-transitions workaround is still the best way around it.

[aendrew]

@yunyu Oh wow, that's a lot of effort — thanks for investigating that, even if there isn't much we can do at the moment.

Maybe it's just worth holding up until we upgrade the library itself to 4.x? That might ultimately be a better use of time — helping with that transition soon after we release 1.x.

Thoughts, @kt3k?

[kt3k]

Thanks for lots of investigations! @yunyu

I agree with @aendrew. It seems better to upgrade d3 first.