Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor Performance #172

Closed
nathancole opened this issue Apr 22, 2014 · 30 comments
Closed

Poor Performance #172

nathancole opened this issue Apr 22, 2014 · 30 comments
Labels
C-performance Category: A issue about performance question R-needs-docs Request for changes: The PR needs documentation

Comments

@nathancole
Copy link

I'm experiencing very poor performance with large datasets. We're loading 10000 values (just random values) and it becomes almost unusable.

@lblb
Copy link

lblb commented Apr 23, 2014

Hi, this is expected, because C3 is based on D3, which uses SVG to render graphics. Every SVG element is created as a separate dom node in browser. Hence the more elements, the more nodes you have to deal with. Consequently performance drops exponential with new elements added. You can find many tests online comparing SVG vs Canvas vs Flash performance and you'll see that SVG is reasonably fast until number of elements is relatively small. You can also get a proof yourself: create a test function that draw and animate 10000 circles alone and you'll see CPU getting hot. You can take a look at this example to get the first impression as well: http://tommykrueger.com/projects/d3tests/performance-test.php

The only way to work with large datasets in SVG library like C3 is to reduce or aggregate values before plotting them. Unfortunately C3 doesn't have any methods for that. My experience is that 500 members in a series is upper limit after which charts become a mess (both visually and performance vice). So it's necessary to design interface of page differently with drill down functionality, so that user could always look for fine details if needed.

p.s.
dc.js is a nice charting library that uses crossfilter for data reduction. (http://nickqizhu.github.io/dc.js/). But it has it's own limitations too.

@masayuki0812
Copy link
Member

I found this issue before and agree with @lblb . It would be difficult to improve the performance dramatically without reducing the number of elements. I'll consider to introduce a feature for reducing or aggregation of data.

@lblb
Copy link

lblb commented Apr 24, 2014

Hi Masayuki, data analysis is a an extensive topic on it own. I'd better suggest focus on charting only and leave that alone. In my case, I do data manipulations on server side and retrieve results on demand. When using knockout.js it's rather easy to bind multiple charts and have them react to data changes. Your functions to load and remove data (and also transform chart to different type) are awesome and work well for me. Other users may prefer retrieving large datasets once and do further manipulations on client side (probably using worker thread so that UI stays responsive). There is no standard approach that fits everyone needs.

I may suggest adding notes to subchart and zoom functions of C3 explaining that they are just experimental and not intelligent at all. User may get a false impression that you can dump megadata to the chart and then zoom in for details.

@masayuki0812
Copy link
Member

@lblb Thank you for the advice. I agree with you and I'll add some notes about the amount of data c3 can process. I know the documentation is completely poor, so I'll add as soon as I can.

@kjavia
Copy link

kjavia commented May 13, 2014

We found Crossfilter.js library (also written by Mike Bostok who wrote D3) to be extremely useful for data aggregation client side. Integrating Crossfilter and C3.js on the same app is pretty easy and no need to add aggregation features in C3.

@masayuki0812
Copy link
Member

Hi @KetanJavia , Thank you for telling. I'll try that library.

@masayuki0812
Copy link
Member

Please try this extension implemented by @danelkhen as #310 . Sample is here:
https://github.com/masayuki0812/c3/blob/master/htdocs/samples/zoom_reduction.html

I think this would a effective solution, so I'll close this issue now.

@danelkhen
Copy link
Contributor

Thanks masayuki0812, if anyone needs help using this feature let me know. Cheers.

@webxl
Copy link

webxl commented Jun 26, 2014

I think this should be reopened. Just because this library uses SVG/d3 doesn't mean it can't handle large data sets performantly.

As a stress test, I have a dataset consisting of 1000 series, with 36 points each. Here are stats from several other SVG-based libraries and C3 (I've turned off points, tooltips, etc. for each chart), running under Canary:

Rickshaw: childCount: 2038, listenerCount: 0, renderTime: 0.26s
Vega: childCount: 4063, listenerCount: 2, renderTime: 1.12s
NVD3: childCount: 39092, listenerCount: 0, renderTime: 4.96s
Highcharts: childCount: 4055, listenerCount: 1, renderTime: 10.25s
C3: childCount: 18354, listenerCount: 3220, renderTime: 26.22s

I don't know if you can't have all the nice interactivity and transition features without the current architecture, but seems like its worth a try. Seems like if I turn off the interactive features, all event listeners should be disabled.

Let me know if you'd like to see my test code.

@masayuki0812
Copy link
Member

Hi, Thank you for looking into this. I think you're right and there must be some room to optimise. We're going to refactor the architecture, so I'll work on this issue in it too.
Could you show us the test code? It would be really helpful if we can see it. Thanks!

@lblb
Copy link

lblb commented Jun 26, 2014

This is interesting, I'm curious to see your testing code as well. It's strange that Rickshaw produced only 2038 child nodes, are you sure that "line smoothing" (interpolation) methods were not active there? This will reduce number of elements actually drawn.

Also please make sure that you disable C3 onresize listeners. By default C3 does a redraw if dimensions of bindto container change. For such a large chart dynamic size scale is definitely a bad idea. I believe all the libraries you compared to has onresize feature disabled (or don't have at all).

@webxl
Copy link

webxl commented Jun 26, 2014

My actual test code isn't really clean and would take a little work to sanitize for public consumption, but here's a fiddle with the basic settings and the helper code that tracks the DOM. The fiddle is just code taken from several files. I didn't actually run all charts on the same page.

http://jsfiddle.net/webxl/nX6Hc/

@webxl
Copy link

webxl commented Jun 26, 2014

@lblb How do I disable C3 onresize listeners?

@lblb
Copy link

lblb commented Jun 26, 2014

Oh, it seems to be hardcoded now:

            // Bind resize event
            if (window.onresize == null) {
                window.onresize = generateResize();
            }
            if (window.onresize.add) {
                window.onresize.add(function () {
                    __onresize.call(c3);
                });
                window.onresize.add(function () {
                    c3.flush();
                });
                window.onresize.add(function () {
                    __onresized.call(c3);
                });
            }

Please try comment out c3.flush(). It should reduce listener count that you observed. Please share your results afterwards

@webxl
Copy link

webxl commented Jun 26, 2014

Commenting out that call yields:

Object {el: div#randomChart.chart.c3, childCount: 18354, listenerCount: 3220, renderTime: "15.42s"}

Better. I thought perhaps my domStats code wasn't tracking listeners accurately, but even delaying the stop() call a second later produces the same count. Why did you expect the listener count to go down?

@lblb
Copy link

lblb commented Jun 26, 2014

I thought it might propagate listeners to internal chart elements, it was just an assumption. Would it be possible to list out what functions are hiding in the number: 3220? It's pretty high, in my opinion. C3 generates a transparent rectangle for each data node, which has on mouseover listeners. It might be that these listeners are still being added, even though legend and tooltip are disabled.

Another aspect of your test is that you generate labels on x axis for every data point. Are you doing the same with other libs in this comparison? Rendering text in SVG is much slower than in native html, it's very likely that most of the 15.42 seconds it's placing labels along the axis (another assumption). You can try tick culling http://c3js.org/samples/axes_x_tick_culling.html option to reduce number of labels, if this is what other chart libs do automatically.

@webxl
Copy link

webxl commented Jun 26, 2014

Adjusting the ticks didn’t help. Here’s the listener breakdown:

listenerCountByClassName:
  c3-event-rect: 216
  c3-legend-item: 3000
  undefined: 4

After commenting out the code that produced those handlers (which could be a bug in itself since all interactivity and legend was disabled), I was still getting slow render times. Almost the same. Event listeners were a red herring. So that led me to the profiler, where I found that 36% of the time was spent on line 1026: pie = d3.layout.pie().value(function (d) {

Once I commented out all pie chart rendering code, I got the render time down to 1.43s. Restoring the legend and event rects listeners pushes that up a little over a second. The unused listeners are probably more of a memory issue than anything.

Looks like the pie chart code is the culprit here.

@lblb
Copy link

lblb commented Jun 26, 2014

Great, that's a good catch! Thanks for this awesome investigation.

@webxl
Copy link

webxl commented Jun 27, 2014

No problem. I think this library has a lot of potential. I'd be happy to contribute in other ways. API docs seem to have holes and once you have unit tests defined, you can start refactoring.

@masayuki0812
Copy link
Member

I appreciate your investigation. This must contribute to the refactoring. Thanks a lot!

@SamMorrowDrums
Copy link

Could we manually elect to disable all animation for complex graphs, and just re-render instead? I think that's causing some of the responsiveness issues.

@ehartford
Copy link

Was the pie chart issue fixed? I didn't see a pull request associated with this issue.

@aendra-rininsland
Copy link
Member

@ehartford That's a good question. It was closed it after #310, after which @webxl did some stress tests. AFAIK, it looks like the pie chart issue is still there.

@masayuki0812 — know if that's resolved? If not, I vote we reopen this and rename it to "Pie chart causing performance issues", or open a separate issue specifically about that.

TBH though, it might be worth postponing until after the animation is refactored using CSS transitions (#774), which should have a really massive impact on performance (and now that I think about it, probably also greatly reduce the number of event listeners because all animation is delegated to CSS).

@ergo
Copy link

ergo commented Jan 9, 2015

Hello Guys,

I'm seeing poor performance on following example:

http://plnkr.co/edit/K1KQmwjL2cgWsROMnBEI?p=preview

This chart can often completly break browser tab. Is there anything that can be done to improve the situation without removing functionality?

@masayuki0812
Copy link
Member

@ehartford @Aendrew Sorry I missed this issue. As you and @webxl said, there was an issue about pie calling. So, I modified this to be called only when it's needed. In my profiler, it looks solved. Could you try the latest code again? I'll release this fix in the next version v0.4.9.

@ergo
I think this fix and padding.left can improve the performance. As it's documented, if you use padding.left, a part of layout calculation will be skipped. Can you try it?

@ergo
Copy link

ergo commented Jan 11, 2015

Maybe I did that incorrectly, but I didn't see any improvement when i added padding to X axis. (Maybe you can take a look at it - I'm still very new to c3js).
I'm wondering if the initial animation is badly influencing the rendering. According to my timing there the whole thing should take ~2s - but it seems to be a lot more.

@ergo
Copy link

ergo commented Jan 11, 2015

http://plnkr.co/edit/UVYSsfaZoPIzHKTwezQH?p=preview

I think my plunk test data could serve as a good test case for performance in general as it seems fairly common thing to plot a year of data with multiple series. After all this is really what subchart is supposed to help ie. improve the navigation over lots of datapoints.

@masayuki0812
Copy link
Member

@ergo You're right. Transition was a bottle neck, so I modified to skip transition completely if transition.duration = 0. So, now the performance should be improved (I don't see the bottle neck in the profiler now) if you set transition.duration = 0. If it's still slow, that's because of the number of elements to draw basically.
I'll release this fix in the next version v0.4.9 shortly.

@masayuki0812
Copy link
Member

v0.4.9 has been released now.

@aendra-rininsland aendra-rininsland added the R-needs-docs Request for changes: The PR needs documentation label Jan 18, 2015
@aendra-rininsland
Copy link
Member

Woo! Adding doc tag to remember to document transition.duration's new behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-performance Category: A issue about performance question R-needs-docs Request for changes: The PR needs documentation
Projects
None yet
Development

No branches or pull requests

10 participants