improve interpolation performance with big input range

[Almouro]

Summary

This drastically improves Animated.interpolate performance when inputRange has a considerable amount of elements (~100 in my tests).

For instance in ActivityIndicator inside react-native-paper, the input has 144 elements https://github.com/callstack/react-native-paper/blob/main/src/components/ActivityIndicator.tsx#L170. react-native-elements has 9k stars, so I'm assuming this is widely used.

Cause

The reason for the performance drop is that if we assume n to be the size of the range, calculating 'inputRange must be monotonically non-decreasing ' + arr essentially calculates arr.toString() which has O(n) complexity.
Since it is recalculated in a for loop, we end up with checkValidInputRange having a O(n²) complexity. Which means ~10k operations if the array has a size close to 100.

Changelog

[General] [Fixed] - Fix performance issue on Animated.interpolate with big input range

Test Plan

Here's a repo reproducing the issue.
The branch fix includes the fix.
Clicking Interpolate runs:

new Animated.Value(0).interpolate({
  inputRange: Array(144)
    .fill()
    .map((_, i) => 1 / (i + 1))
    .reverse(),
  outputRange: Array(144)
    .fill()
    .map((_, i) => 1 / (i + 1))

Here's a comparison of JS thread perf before the fix and after the fix:

• on a Samsung J3 2017 (lower end)
• using Flipper and https://github.com/bamlab/react-native-performance)
• __DEV__ mode deactivated
• clicking the button and waiting 15s

Before	After

The error still throws if inputRange is incorrect:

However if __DEV__ mode is deactivated, no error is thrown

[javache]

Thanks for spotting this! I think all of the sanity checks in this file should probably be disabled in release builds. That will also enable dead code stripping to remove these checks.

Can you add an if (__DEV__) { check around the calls to checkInfiniteRange / checkValidInputRange and other invariants inside createInterpolation instead?

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: bfc3935
Branch: main

[Almouro]

Hi @javache, thanks!

It feels to me like this could warrant a bigger discussion on invariant in release in general. What do you think?

• One pro of having them in release is they pop up on crash reporting tools such as Sentry, and can useful to analyze bugs.
• On the other hand, keeping them can be bad for performance, so if we go this route here, I'd be happy to codemod it and surround every invariant in RN by if (__DEV__)

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,767,516	-562
android	hermes	armeabi-v7a	7,171,437	-577
android	hermes	x86	8,073,746	-581
android	hermes	x86_64	8,054,066	-570
android	jsc	arm64-v8a	9,645,100	-179
android	jsc	armeabi-v7a	8,419,461	-183
android	jsc	x86	9,594,319	-175
android	jsc	x86_64	10,191,730	-191

Base commit: bfc3935
Branch: main

[javache]

Some invariants are important to keep around in production, because there may be states we really don't want the application to get into and do want to see escalated to crash reporting.

Other invariants, like these ones, in my opinion, are more about catching developer errors, and can be skipped in release. The trade-off here is that we may miss some errors if the inputs to the interpolate call are not static, but I think that's reasonable here.

[Almouro]

Cool, thanks for the explanation! 🙏
Added the __DEV__ wrapping as well then

[javache]

Can you make this + String(arr) and remove the FlowFixMe statement?

[facebook-github-bot]

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @Almouro in f503b21.

^{When will my fix make it into a release? | Upcoming Releases}