Victory-native: Performance for victory-native

[boygirl]

We've been seeing a bunch of related issues for performance in victory-native and especially for container components like VictoryZoomContainer

We recently completed a performance audit in which was focused specifically on VictoryZoomContainer as a test case for this issue:

From @michaelmerrill who performed the audit:

This review focused solely on the victory-zoom-container and its performance while “pinching” to either zoom in or out on a chart. My assumption going into this audit was that there would be wasted renders that we could eliminate to improve performance and reduce lag. To test that theory, I replaced the demo app with a simple VictoryChart that used VictoryZoomContainer as a container component and used four very simple data points to reduce complexity. I also logged each victory-native component for updates and logged incoming props in the components that updated to double check that the re-renders were accurate. Looking at the logs I did not see anything abnormal. The shouldComponentUpdates were catching most of the issues and components weren’t being passed anything they didn’t need. There was also already a throttling function wrapping the pinching logic that is helping reduce the renders.

Pinching/moving is hard to test since I can’t be sure how many events will be sent with each action (and I can’t always repeat the exact same action), but with the most minimal zoom action, I saw a substantial number of requests calling into the react-native-svg package. Without having looked into the react-native-svg, I believe the zoom performance issue is downstream of the victory-native package. I would be interested to see how many requests are crossing the bridge into the native layer and how expensive the operation is to redraw each node.

This is the current state of this issue. We will continue looking for ways to improve performance, but we do not have any currently actionable changes in either repo.

I am going to close several related performance issues and reference them in this PR so that there is a single place for issues related to performance.

[arpitjain-in]

hi @boygirl ... is there any ETA for all these item?

[boygirl]

@arpitjain-in if you read the issue text above you will see that we spent a couple of days worth of developer time last week trying to track down the cause of performance problems in victory-native, and we were unable to identify anything actionable. Without any actionable problems it isn't possible to offer an ETA for a solution. This is a frustrating situation for us as well. Thank you for your continued patience.

[Luomusha]

[arpitjain-in]

@boygirl Thanks for your response on my previous comment. Here I would like to explain my scenario where I could see the very noticeable lag which is hitting performance very badly.

I'm having area chart with two color variant... Above x axis - area colored with green color and below to axis colored with red color. Having dataset of 96 hours(so 96 ticks) available on x axis. Given 1200px as width and horizontal scroll as true.

I'm enabling/disabling VictoryCursorContainer on long press event and on normal press - enabling horizontal scroll.

Having said above scenario , once I'm doing long press to read dataset value using VictoryCursorContainer, screen becomes super slow and I can see tooo much lag in moving VictoryCursor.

Please suggest on this.

Thanks

[arpitjain-in]

@boygirl any update on above comment ?

[jpdriver]

Investigation update

tl:dr there doesn't appear to be an easy way to fix this at this time. It's an unintended side-effect of the way that Events are dispatched within any component that uses a <VictoryContainer />.

Overview

• Each SVG element within a <VictoryChart /> can have it's own Events (e.g. each bar in a <VictoryBar /> can have it's own onClick)
• Each Event can target any node within the same <VictoryChart />, regardless of whether or not that node is a direct descendent / parent of the node dispatching the event
• In order to do this, these Events are passed to a parent HOC called VictorySharedEvents
• Within Charts that utilise a VictoryContainer (e.g. <VictoryZoomContainer />), VictorySharedEvents is responsible for rendering the Container, the child Victory Primitives (e.g. each individual <Bar />), and ultimately the SVGs (react-native-svg)

The Problem

• any interaction with a Chart implementing a VictoryContainer relies on the ability for VictorySharedEvents to mutate the props passed down to the Container
• Mutating the props in VictorySharedEvents is therefore causing cascading re-renders of the Container, the Victory Primitives, and ultimately the SVGs
• In some cases, these cascades are potentially desired behaviour (e.g. zooming into a Chart needs to continuously mutate the visible domain and re-render the SVGs accordingly)
• Because Events can target any other element within the same <VictoryChart />, it's possible that at various levels in the tree neither the props nor state will have changed, until the target level is reached
• This appears to make adding additional shouldComponentUpdate checks to catch wasted re-renders impossible; as returning false for any levels in the hierarchy will break the mutation chain, potentially causing some Events to never reach their intended target level

Additional Info

• Not all SVGs are affected equally
• Given the same random sample data set of 1000 points, a <VictoryBar /> performed significantly worse when zooming than a <VictoryLine />. This is because the latter is composed of fewer Victory Primitives and react-native-svg does not need to re-render so many SVGs
• There is no minimum threshold for events (i.e. as soon as a pinch gesture is detected we begin repeatedly calling the handler for any X or Y axes value change. For example this could be +/-0.000001px)

DEMO

• I created a minimal demo app which illustrates this problem by injecting some additional logs using patch-package
• This adds shouldComponentUpdate checks using react-fast-compare at several points; but only with the intention of logging changes to show wasted re-renders

Examples

1. A single click on the demo app

• This causes 6x re-renders
• There is no implemented onClick handler in user space; this is purely caused by the <VictoryZoomContainer />

• Closer inspection of the logs reveals a repeating cycle. The <VictoryChart /> re-renders 3x, with each Chart re-render re-rendering VictorySharedEvents 2x each.
  

• Note the propsHaveChanged and stateHasChanged values

• On the second VictorySharedEvents render, neither value was true
• If we introduce a shouldComponentUpdate check here, zooming will break

---

1. Moving a <ScrollView /> containing the same Chart

• if the user initiates a scroll over the Chart, it will continuously re-render despite the visible domain remaining unchanged

---

1. A single click on a <VictoryBar /> demo, showing a cascade of 27x VictorySharedEvents re-renders, despite no visible domain change (and again, no desired user space onClick handler)

---

1. React Profile showing work performed when zooming a <VictoryBar /> chart

[Eyesonly88]

FYI latest version of react-native-svg (9.0.0) has major speed improvements such as https://github.com/react-native-community/react-native-svg/issues/894

@boygirl Is it possible to update the react-native-svg dependency to 9.0.0?

Thanks.

[boygirl]

@Eyesonly88 victory-native doesn't have a dependency on react-native-svg, only a peerDependency. I just checked the latest version of react-native-svg with the latest version of victory-native and I don't see any breaking changes in the latest version for victory-native

[Eyesonly88]

Awesome, thanks for the quick reply and the test.

I'll update the dependency and report any improvements bach here.

[ericketts]

@Eyesonly88 should I take your lack of update as being indicative of no improvement? the linked ticket here seemed related to svg lib startup time, not necessarily anything about the rendering in the lib..

[nonewcode]

noticing some really poor performance when using panning. Great library but for someone needing some real time chart movement, i may have to look for another option.

[Thembelani]

Hi @joshuakelly did you find any viable alternatives?

[JulianKingman]

@Thembelani I love the api of victory native, but have concluded that its architecture doesn’t match well with RN (crosses the bridge too many times). I’m switching to react-native-charts-wrapper, which wraps the most popular native chart libraries, it should work much better.
Maybe in the future victory native can be refactored with reanimated or something so it’s not so expensive performance wise.

_{Sent with GitHawk}

[andrei-zgirvaci]

I was having performance issues with VictoryZoomContainer component for 1 year. I finally found a solution 😲. In the official documentation, there is a section where it is described how to optimize VictoryZoomContainer. After I implemented it, my chart is blazing fast 🚀, even on a simulator. Try it out, you won't regret it!!! 🎉
https://formidable.com/open-source/victory/guides/zoom-on-large-datasets

[edreyyo]

Thanks for all the effort put into Victory Native @boygirl and team 💚

A performance related question: is it possible to recreate the smooth behaviour show in the GIF below using Victory Native?

For example, having a cursor/point follow the contours of VictoryArea graph exactly, even though there aren't many x-values (dates, in this case), instead of "jumping" between distinct x-values.

[nonewcode]

@Thembelani i'm considering https://github.com/JesperLekland/react-native-svg-charts

There's not too many viable alternatives that give such an easy implementation.

[JulianKingman]

@codecog I tried that library a year or two ago, and it was buggy with mediocre support. May be different now, but it really wasn’t fun to work with...

_{Sent with GitHawk}

[outaTiME]

it will great to have PanGestureHandler from react-native-gesture-handler with react-native-reanimated for smooth animation at 60fps, here an awesome video from William Candillon.

https://www.youtube.com/watch?v=uSucm5jjkRI

Any plan on roadmap? thanks !!!

[nonewcode]

Could be a good idea to have a look into a fundamental change under the hood for some of the apis. I'm not sure how much effort it would be but I see victory native as the best charting library to go with for RN projects and there's still room for improvement

[outaTiME]

in some how related to: #584 🤔

[outaTiME]

Awesome cursor with reanimated 2

https://github.com/rainbow-me/react-native-animated-charts

[vshjxyz]

Hello, I'm facing the same severe slowness with VictoryCursorContainer even on an empty chart that just has the cursor

  <VictoryContainer
        containerComponent={<VictoryCursorContainer cursorDimension="x" />}
      >

This is less noticeable (but still laggy) in iOs. Although the main issue for me is the Android side, where weird things happen. I've tried with RN 0.60, 0.62 and 0.63 and they all present the same issue. The graph animation AND VictoryCursorContainer are terribly slowed down (perf monitor shows 2-8fps) while scrolling, both with hermes enabled or disabled.

The weirdest thing is that when enabling debug mode, the performance of both the chart animation AND the cursor are bumped up to 35-50fps (!!!) while on iOs the other way around happens as expected (while debugging, things are slower). Has anyone got a clue on this? it should be visible with a simple graph even without datapoints, if you need more info to reproduce, let me know.

[edreyyo]

@vshjxyz I think victory-native crosses the JavaScript-to-native bridge too often when using the panning gesture so performance will always be poor, even when using a very basic example.

I ended up using react-native-svg-charts to get decent panning performance, although it doesn't support these gestures out-of-the-box.

victory-native is very powerful but unfortunately has terrible performance on RN.

[tombohub]

I can confirm it's slow for normal React. Candlestick chart zooming and panning it's slow.

[mikkovor]

Hello, I'm facing the same severe slowness with VictoryCursorContainer even on an empty chart that just has the cursor

  <VictoryContainer
        containerComponent={<VictoryCursorContainer cursorDimension="x" />}
      >

This is less noticeable (but still laggy) in iOs. Although the main issue for me is the Android side, where weird things happen. I've tried with RN 0.60, 0.62 and 0.63 and they all present the same issue. The graph animation AND VictoryCursorContainer are terribly slowed down (perf monitor shows 2-8fps) while scrolling, both with hermes enabled or disabled.

The weirdest thing is that when enabling debug mode, the performance of both the chart animation AND the cursor are bumped up to 35-50fps (!!!) while on iOs the other way around happens as expected (while debugging, things are slower). Has anyone got a clue on this? it should be visible with a simple graph even without datapoints, if you need more info to reproduce, let me know.

Using android I am getting the exact same problem with VictoryVoronoiContainer. With debug mode on it's around 45-55 fps, feels smooth but without debug it drops to 2-4 fps which is obviously unacceptable. Dataset is around 400-700 and nothing seems to make it work properly.

[vshjxyz]

Any reason for closing this one? I think it's THE major drawback of victory-native and the (only) reason why we'll be migrating away in the near future

[JulianKingman]

Agreed. I love this library and API, but the performance makes it unusable in real-world situations.

[edreyyo]

Is Victory Native no longer a priority of Formidable Labs, @boygirl? I absolutely love the package but the performance issues relating to any kind of interactivity or animations result in poor UX.

[boygirl]

It's a tough situation. Victory Native is and always has been a _very_ light layer over Victory. Really all the package does is swap out the rendered SVG elements for RN-SVG elements, and some PanResponder additions to container components. There are few interventions I can think of to make to Victory Native itself that would improve the situation. I am definitely open to input here, but I don't have anything actionable. I can re-open this issue for tracking, if people would find that helpful.