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

Realtime LineChart poor performance due to calcMinMax #3166

Open
codingspark opened this issue Jan 9, 2018 · 7 comments
Open

Realtime LineChart poor performance due to calcMinMax #3166

codingspark opened this issue Jan 9, 2018 · 7 comments

Comments

@codingspark
Copy link

I've found a possible performance bottleneck. In a realtime chart Adding many entries per second to the chart I noticed that automatically the calcMinMax of the dataset is called just after EACH values.append(e)

calcMinMax function determine the min and max values using .forEach that is causing the CPU over 60% spent looping the values, moreover in the main thread.

I'm performing some experiment disabling min/max calculation or using native for loop instead of array .forEach

@danielgindi please have a look, is really calcMinMax on all values needed? I calculate min/max Y values manually in my code and only on visible values, so maybe you could expose some boolean to enable/disable min/max auto calculation and improving performance removing the forEach function and avoid function calls.

```
open override func calcMinMax()
{
    guard !values.isEmpty else { return }

    _yMax = -Double.greatestFiniteMagnitude
    _yMin = Double.greatestFiniteMagnitude
    _xMax = -Double.greatestFiniteMagnitude
    _xMin = Double.greatestFiniteMagnitude

    values.forEach { calcMinMax(entry: $0) }
}
  
@codingspark
Copy link
Author

The DataSet implementation in Android is different.

in swift the didSet is invoked even when we add a new value, instead on Android we have separated addEntry, setValues methods.

when we add a new value Android updates the min/max comparing just the added entry not the whole array. when removing entries the min/max are recalculated on full-array.

Check
https://github.com/PhilJay/MPAndroidChart/blob/master/MPChartLib/src/main/java/com/github/mikephil/charting/data/DataSet.java

@jjatie
Copy link
Collaborator

jjatie commented Jan 9, 2018

We have those methods too. I think we should be treating DataSet as a collection and hide it's backing store (aka values). All side affects should be baked into those methods and not be inside the setter for values. In the short term this is an easy fix, but we are currently unable to implement this using the standard Swift way because of conflicts with Objective-C compatibility.

I think there is some more we can do to improve performance with live data. I'll try to find my notes on it this week.

@codingspark
Copy link
Author

Using Instruments you can discover the full calls stack causing the CPU overload.
I'm adding 30 entries per second and I have 10 minutes of data buffer that means 30 * 60 * 10 = 18000 entries to loop.

replacing the .forEach is a good starting point.

screen shot 2018-01-09 at 12 16 00

@jjatie
Copy link
Collaborator

jjatie commented Jan 11, 2018

@codingspark Could you evaluate the algorithm that's used. It's in 3 spots, ChartView, ChartData, and ChartDataset

@jpm-polymorph
Copy link

Updating from v3.0.4 to v3.0.5 my graphs are now unusably slow, and as far as I can see it is due to calcMinMax(entry: e) being called every time I addEntry. Or perhaps due to the notifyDataSetChanged() in the didSet of values.

A function to use to add a historical data point, without calculating anything, would be useful.

@jjatie
Copy link
Collaborator

jjatie commented Jan 24, 2018

This will be added back in 3.1

@jjatie
Copy link
Collaborator

jjatie commented Jan 26, 2018

I am moving 3.1 back to the old calculation method to restore the old performance characteristics. The current performance issues were an oversight on my part. 3.1 will be released next week.

In a future update, live data performance will be further improved.

Given that any future discussion will be about another version, a new issue should be opened. If this is regarding a feature request, use #29

@ChartsOrg ChartsOrg locked as resolved and limited conversation to collaborators Jan 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants