Open cdcv opened 5 years ago
For others who may run into this same problem, these settings significantly improved render time performance:
<LineChart
withDots = {false}
withShadow = {false}
withInnerLines = {false}
withOuterLines = {false}
Hey @cdcv, I know it's been a while since you posted but I was interest in doing some work on this. When you said '200 Elements' was that 200 data points as in:
const data = {
datasets: [
{
data: [1,2,3,4,5,.....................,200],
color: (opacity = 1) => `rgba(134, 65, 244, ${opacity})`
}
Also, when you were talking about real-time data processing, were you attempting to update the line chart in real-time or the lag on the chart just meant that the rest of your app couldn't process the data in real-time since it was blocked up? Thanks!
Glad you’re going to work on it. Great! It’s been a while, but here’s my recollection. I believe that the issue was that when rendering a dataset with a large number of values (as you indicate) it would block the rest of the app for quite a while. Already posted this, but it was substantially improved by the parameters chosen. Current biggest problems, if you’re working on the repo: 1) If data passed isn’t as expected (eg null) it crashes the app.  Error checking / graceful recovery / default parms would be very helpful. 2) Background fades don’t work as expected on some platforms. 3) There is some weird error (there should never have been anything to do with onPress related to the chart) that produces this long stacktrace:
bundle.js:264632 Warning: Unknown event handler property onPress
. It will be ignored. in circle (created by Circle) in Circle (at scoreChart.js:73) in g (created by G) in G (at scoreChart.js:362) in g (created by G) in G (at scoreChart.js:278) in svg (created by Svg) in Svg (at scoreChart.js:274) in div (created by View) in View (at scoreChart.js:273) in ScoreChart (at AppChart.tsx:44) in div (created by View) in View (at AppChart.tsx:40) in AppChart (at SessionInsights.tsx:135) in div (created by View) in View (at SessionInsights.tsx:135) in div (created by View) in View (created by ScrollView) in div (created by View) in View (created by ScrollViewBase) in ScrollViewBase (created by ScrollView) in ScrollView (at SessionInsights.tsx:124) in div (created by View) in View (at SessionInsights.tsx:120) in div (created by View) in View (created by ImageBackground) in ImageBackground (at DisplayAppBackground.tsx:9) in DisplayAppBackground (at SessionInsights.tsx:120) in SessionInsightsScreen (at SceneView.js:9) in SceneView (at StackViewLayout.tsx:899) in div (created by View) in View (created by AnimatedComponent) in AnimatedComponent (at StackViewCard.tsx:93) in div (created by View) in View (created by AnimatedComponent) in AnimatedComponent (at StackViewCard.tsx:80) in Card (at createPointerEventsContainer.tsx:95) in Container (at StackViewLayout.tsx:971) in div (created by View) in View (at StackViewLayout.tsx:383) in div (created by View) in View (created by AnimatedComponent) in AnimatedComponent (at StackViewLayout.tsx:379) in UnimplementedGestureHandler (at createHandler.web.js:201) in PanGestureHandler (at StackViewLayout.tsx:372) in StackViewLayout (at withOrientation.js:30) in withOrientation (at StackView.tsx:103) in div (created by View) in View (at Transitioner.tsx:267) in Transitioner (at StackView.tsx:40) in StackView (at createNavigator.js:61) in Navigator (at SceneView.js:9) in SceneView (at StackViewLayout.tsx:899) in div (created by View) in View (created by AnimatedComponent) in AnimatedComponent (at StackViewCard.tsx:93) in div (created by View) in View (created by AnimatedComponent) in AnimatedComponent (at StackViewCard.tsx:80) in Card (at createPointerEventsContainer.tsx:95) in Container (at StackViewLayout.tsx:971) in div (created by View) in View (at StackViewLayout.tsx:383) in div (created by View) in View (created by AnimatedComponent) in AnimatedComponent (at StackViewLayout.tsx:379) in UnimplementedGestureHandler (at createHandler.web.js:201) in PanGestureHandler (at StackViewLayout.tsx:372) in StackViewLayout (at withOrientation.js:30) in withOrientation (at StackView.tsx:103) in div (created by View) in View (at Transitioner.tsx:267) in Transitioner (at StackView.tsx:40) in StackView (at createNavigator.js:61) in Navigator (at createAppContainer.js:348) in NavigationContainer (at App.js:284) in div (created by View) in View (at App.js:281) in AppControl (at App.js:279) in ErrorBoundary (at App.js:278) in App (at withExpoRoot.web.js:8) in RootErrorBoundary (at withExpoRoot.web.js:7) in ExpoRootComponent (at registerRootComponent.web.js:7) in RootComponent in div (created by View) in View (created by AppContainer) in div (created by View) in View (created by AppContainer) in AppContainer
Thanks! On Dec 21, 2019, 5:18 PM -0300, SimonPringleWallace notifications@github.com, wrote:
Hey @cdcv, I know it's been a while since you posted but I was interest in doing some work on this. When you said '200 Elements' was that 200 data points as in: const data = { datasets: [ { data: [1,2,3,4,5,.....................,200], color: (opacity = 1) =>
rgba(134, 65, 244, ${opacity})
} Also, when you were talking about real-time data processing, were you attempting to update the line chart in real-time or the lag on the chart just meant that the rest of your app couldn't process the data in real-time since it was blocked up? Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.
Thanks! This is great info. Currently, using React's Profiler API, I'm seeing render times in this situation of a little over 2s. Does that sound like about what you were experiencing?
After further investigation, it seems that the issue stems from the ways keys are being used throughout the application but specifically in Line-Chart. React uses this key during future renders to compare elements within the DOM and right now LineChart isn't architected to take advantage of this feature in React - this article here does a much better job of explaining the core issue than I could https://medium.com/blackrock-engineering/5-common-mistakes-with-keys-in-react-b86e82020052. For sake of example, let's focus on dots specifically. This code here from renderDots presents an issue
output.push(
<Circle
key={Math.random()}
cx={cx}
cy={cy}
...
/>,
In order to help avoid wasteful re-renders, React will compare the keys of child elements within parent, if the keys of a rendered component and a to-be-rendered component are the same, then React will either update the changed info for that component, or leave it alone if it is unchanged. Our problem is that, since we're using Math.random() to generate keys for the dots, the keys are different every time the function to generate those elements gets called (each re-render of LineChart) meaning that we're removing and re-rendering every dot on the chart, rather than just the new additions as new data gets added to the graph. A quick solution for this would be to use a value for key that wouldn't change between re-renders, and a common option is to use the index of the value currently being mapped over. This exposes a second issue which is that React compares all keys within a parent element. Because we are mapping over dataSets to create dots and lines all within the same parent (LineChart), child elements (dots and lines) would have the same keys if we used their index in the dataSet and that would cause further issues. I'm working up a solution and, so far have transitioned dots to their own component, where we can use indexes as keys since the dots are now wrapped within their own parent component. So far, while this doesn't reduce the time of the first render of LineChart with 400 data points(~2s), it has shaved between 1-1.5 seconds off of the re-renders when adding an additional data point every 3s (to simulate real-time updates). This will likely take some time to complete, but I estimate I'll be able to have something ready for PR for LineChart in the next 2 weeks or so.
I have a problem on the barchat I need to remove the decimal places
Extremely slow rendering, multiple seconds. Unreasonably slow... sadly, as I had already written all the charts.
After further investigation, it seems that the issue stems from the ways keys are being used throughout the application but specifically in Line-Chart. React uses this key during future renders to compare elements within the DOM and right now LineChart isn't architected to take advantage of this feature in React - this article here does a much better job of explaining the core issue than I could https://medium.com/blackrock-engineering/5-common-mistakes-with-keys-in-react-b86e82020052. For sake of example, let's focus on dots specifically. This code here from renderDots presents an issue
output.push( <Circle key={Math.random()} cx={cx} cy={cy} ... />,
In order to help avoid wasteful re-renders, React will compare the keys of child elements within parent, if the keys of a rendered component and a to-be-rendered component are the same, then React will either update the changed info for that component, or leave it alone if it is unchanged. Our problem is that, since we're using Math.random() to generate keys for the dots, the keys are different every time the function to generate those elements gets called (each re-render of LineChart) meaning that we're removing and re-rendering every dot on the chart, rather than just the new additions as new data gets added to the graph. A quick solution for this would be to use a value for key that wouldn't change between re-renders, and a common option is to use the index of the value currently being mapped over. This exposes a second issue which is that React compares all keys within a parent element. Because we are mapping over dataSets to create dots and lines all within the same parent (LineChart), child elements (dots and lines) would have the same keys if we used their index in the dataSet and that would cause further issues. I'm working up a solution and, so far have transitioned dots to their own component, where we can use indexes as keys since the dots are now wrapped within their own parent component. So far, while this doesn't reduce the time of the first render of LineChart with 400 data points(~2s), it has shaved between 1-1.5 seconds off of the re-renders when adding an additional data point every 3s (to simulate real-time updates). This will likely take some time to complete, but I estimate I'll be able to have something ready for PR for LineChart in the next 2 weeks or so.
Please submit your PR, thanks!
I am having the same problem. rendering 280 points takes 3 seconds!
It's really bad, rendering a single graph with only 10 points freezes my app for a bit. This library seems incredibly unoptimized in multiple areas.
Edit: It appears this library is no longer maintained. I would recommend using Victory Native, as it has better performance and features.
The time to render a line chart with 200 elements on a newish Android Pixel 3XL is... seconds!!! Thanks very much for this lovely library. That said... this issue currently makes it impossible for us to use. For a real-time data processing app, this means that the whole app is blocked (and can't gather data) during that render period == very bad. Can someone suggest what line chart settings can be used that might substantially speed up the rendering of the chart? I've already tried withDots={false} and subsampling my data so that it's less points to plot. Thank you.