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

Attaching (for example creating Animated.Value or using cond, eq, set, etc) and detaching nodes blocks JS thread #194

Closed
terrysahaidak opened this issue Feb 24, 2019 · 13 comments
Assignees

Comments

@terrysahaidak
Copy link
Contributor

terrysahaidak commented Feb 24, 2019

Updated issue:

Reanimated uses a bridge to interact with the native side. Hight usage of bridge (lots of calls with lots of data to be passed) blocks the thread. Reanimated doing lots of calls. Even simple animation may produce 500-1500 calls. This is huge amount of calls. The main problem that it totally blocks the UI, the app might be showing just a blank screen or freeze until all the nodes are created and connected to each other.

You can verify it by using this simple snippet in your App.js (works only with React Native CLI project):

import MessageQueue from 'react-native/Libraries/BatchedBridge/MessageQueue.js';

let count = 0;
const spyFunction = msg => {
  if (msg.module === 'ReanimatedModule') {
    console.log(++count, msg);
  }
};

MessageQueue.spy(spyFunction);

Most of the calls are connectNodes.

There is a thing which may reduce some amount of the calls – proc node. But it does not fix it for low-end devices;

You can verify the delay of rendering everything using this example:
https://snack.expo.io/@terrysahaidak/reanimated-header

Original issue

I'm building a library which helps define an animation without any knowledge of reanimated. It's inspired a lot by Animatable and called react-native-reanimatable.

It relies on Animated.Code a lot since mostly I'm triggering animation by some props (changing value which runs/stops clocks).

But I'm noticed if I'm using several Animated.Code, it creates animated nodes and blocks JS Thread. It's noticeable on both Android and iOS.

You can check it out here (on my Xiaomi Mi5 and iPhone 7) with the Expo SDK 32:
https://drive.google.com/open?id=1RUaVYcP6-VG332rGS7lA5RsyRXJh7VTP
https://drive.google.com/open?id=15LDQphmnYyZtajFCERuugPgpUmeOIdG3

You can reproduce it using my lib example app:
https://github.com/terrysahaidak/react-native-reanimatable/tree/master/Example

Navigate to Base transition. It renders 10 components with Animated.Code under the hood. You can change count here:
https://github.com/terrysahaidak/react-native-reanimatable/blob/master/Example/screens/TransitionBase/TransitionBase.js#L103

Also here is some underlying logic:
Creating animation: https://github.com/terrysahaidak/react-native-reanimatable/blob/master/lib/core/createTransitionAnimation.js
Using of Animated.Code: https://github.com/terrysahaidak/react-native-reanimatable/blob/master/lib/components/TransitionAnimation.js

Thank you for such a great library!

UPDATE:

I've figured out that Animation.Code itself doesn't block the thread, but creating animation values and nodes do.

@terrysahaidak terrysahaidak changed the title Attaching and detaching nodes within Animated.Code blocks JS thread Attaching (for example creating Animated.Value) and detaching nodes blocks JS thread Mar 24, 2019
@terrysahaidak terrysahaidak changed the title Attaching (for example creating Animated.Value) and detaching nodes blocks JS thread Attaching (for example creating Animated.Value or using cond, eq, set, etc) and detaching nodes blocks JS thread Jun 9, 2019
@sintylapse
Copy link

@terrysahaidak does this still an issue?

@terrysahaidak
Copy link
Contributor Author

Hi @sintylapse, yes, it is. And it will be an issue untill reanimated migrate to jsi and react native use fabric.

But with 1.3 and proc nodes it might make it less obvious for the user to catch, so performance should be better, haven't tried it yet though.

@computerjazz
Copy link

I'm running into this too (esp on Android) when adding semi-complex animated values and clock/springs to FlatList items. I've been experimenting with proc and didn't see huge gains until I also copied over the betterSpring proc from the example: https://github.com/kmagiera/react-native-reanimated/blob/master/Example/test/index.js#L27-L62

I am still seeing some JS blocking, but definitely an improvement.

@terrysahaidak
Copy link
Contributor Author

@computerjazz make sure you have ran the animation or just imported that file which has a proc defined before rendering a bunch of items in the flat list. also, make sure you've been defining procs outside the render, they should be static, only then they can be reused and won't block a js thread.

@michalsek michalsek added 🐞bug-bash-dec19 Missing repro This issue need minimum repro scenario labels Dec 19, 2019
@michalsek
Copy link
Member

Hi @terrysahaidak, thanks for reporting the issue, could you provide minimal code example that reproduces the problem?

@terrysahaidak
Copy link
Contributor Author

terrysahaidak commented Dec 19, 2019

Hi @michalsek, did you check out my original comment here? The problem not with my library I've used as an example here.

The problem is that reanimated does too many calls to the bridge. You can easily count them using this snippet with any example:

import MessageQueue from 'react-native/Libraries/BatchedBridge/MessageQueue.js';

let count = 0;
const spyFunction = msg => {
  if (msg.module === 'ReanimatedModule') {
    console.log(++count, msg);
  }
};

MessageQueue.spy(spyFunction);

For simply animation it generates thousands of calls where React Native Animated produces 100-200. This is a huge difference. You may not notice it on iOS, but on android, even on fast phone like my pixel 3 it is noticable.

We've tried already to fix it using JSI, but it didn't really help. @chrfalch made a fork for iOS where it was 2-3 faster. But there is almost no problem with it on iOS. My fork with JSI on android was 30% faster. This is just nothing.

Also to fix it @chrfalch created proc node. It does reduce amount of nodes created. But still, unfortunately, on low-end android it's unusable for most of the cases.

@kmagiera as well as @osdnk are aware of this problem.

@euroclydon37
Copy link

I'm wondering what the status of this problem is. I know proc is a big saver in a lot of circumstances. But you can't pass arrays to it, meaning you can't fix interpolate which is causing huge performance issues for me because it's in a component that appears alot. 118 components rendered together using interpolateColor from react-native-redash, creates like 13K bridge messages.

@terrysahaidak
Copy link
Contributor Author

Everything I can say so far there is work in progress to fix this! And it's really exciting!

I hope it will be released soon!

@euroclydon37
Copy link

Nice! Is there a place I can follow that work?

@Garkavenko
Copy link

Everything I can say so far there is work in progress to fix this! And it's really exciting!

I hope it will be released soon!

@terrysahaidak Could you say, how long will it take to be done(more or less than month :) )?

@slorber
Copy link
Contributor

slorber commented May 22, 2020

For the ones interested:

image

https://twitter.com/kzzzf/status/1263157145572442112


Also, I've had similar problems on complex animation graphs.
I wasn't really able to optimize much the mount phase, but if you memoize the graph aggressively (ie, avoid creating new node instances), I've found updates/re-renders are less expensive. Didn't measure the bridge calls but I guess it helps.

@jakub-gonet jakub-gonet self-assigned this Jul 22, 2020
@jakub-gonet
Copy link
Member

Most issues are connected with initialization/deinitialization and we can't really avoid this because Reanimated 1 is based on animations composed on nodes, so we have to push every node through the bridge.
Updates can be avoided in ways that @slorber mentioned (e.g. using useValue hook, using proc, some micro-optimizations like avoiding negative conditions in cond node and using else block, etc) and most of the time they're not problematic.
Some solutions were proposed in the past (most notably creating jsNode - you send js function as a string and animation is evaluated on JS VM in the native side) but the problem lies in the bridge traffic and this can't be removed completely.

I suggest following the development of Reanimated 2 (it's pretty stable at this moment) and maybe giving it a try - it's way easier to write animations in v2 than in v1.

I'm going to close this issue as there's no practical solution to it.
I'm open for discussion though.

@jakub-gonet jakub-gonet added 🐞bug-bash-dec19 and removed 🐞bug-bash-dec19 Missing repro This issue need minimum repro scenario labels Jul 22, 2020
@terrysahaidak
Copy link
Contributor Author

As someone who's been using Reanimated 2 from times way before the actual release I can confirm it fixes all the issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants