React-native: Experimental Navigator push doesn't work well on Android 5.0.1

Created on 30 Mar 2016  路  10Comments  路  Source: facebook/react-native

I am running the UIExplorer example from the stable 0.22 branch, below are my steps to replicate

Running the following commands on osx 10.11.3

git clone -b 0.22-stable --single-branch --depth 1 https://github.com/facebook/react-native.git
cd react-native
npm install
./gradlew :Examples:UIExplorer:android:app:installDebug
adb reverse tcp:8081 tcp:8081
./packager/packager.sh

Then starting UI explorer on my phone (Samsung GT-I9505 - 5.0.1), go to Navigation (Experimental) -> Animated Example -> Push! causes an extremely jerky animation and then the phone locks up and eventually crashes.

Note: I am not using chrome debugging.

The following is a CPU usage graph
image
The red line is navigating to the animated example, the blue line is after tapping push for the first time.

This also happens if I build the release version, sign it with my personal key and install it on the device.

I believe this issue is related to the following issues but cannot say for certain
https://github.com/facebook/react-native/issues/6666
https://github.com/aksonov/react-native-router-flux/issues/355
https://github.com/aksonov/react-native-router-flux/issues/401
https://github.com/facebook/react-native/issues/6477
https://github.com/facebook/react-native/issues/6474

I'm tagging @satya164 @ericvicenti @hedgerwang as I believe they may be able to help out (sorry if I got that wrong!)

Locked

Most helpful comment

FYI, this issue was hitting me hard and I believe I've fixed it by changing this line here:

https://github.com/facebook/react-native/commit/28649b8cf0a58fe4f32016a7ce6de1e35b349d2b#diff-04c37ff76f85aea35e8475617539b75fL251

and here

https://github.com/facebook/react-native/commit/28649b8cf0a58fe4f32016a7ce6de1e35b349d2b#diff-04c37ff76f85aea35e8475617539b75fL156

There was a bug where the animated values weren't actually being unsubscribed to:

this._layoutListeners.forEach(subscription => subscription.remove);

should become

this._layoutListeners.forEach(subscription => subscription.remove());

This is fixed in master.... but I'm not sure if it affects 0.23 or not. Also, others have mentioned that there is still a problem on devices even in 0.23. There may be more issues that are causing this, but this was definitely a big part of it.

cc @hedgerwang @ericvicenti

All 10 comments

I've also seen this issue. Good stuff.

Testing this on 0.23-stable shows significant improvement (It doesn't crash after the first push), but after a few pushes (~20) performance degrades significantly, taking up to 10 seconds to respond to any input. This doesn't seem to be the case in Genymotion on any version (4.4.4/5.0.0/6.0.0) on 0.23

Can you try using chrome profiler to see what's happening?

@thessem : did you mean that this can't be reproduced in Genymotion (simulator), but needs a real device instead?

@hedgerwang Both issues are significantly worse on device compared to Genymotion, On simulator the 0.22.2 crash still appears after 2/3 pushes, the 0.23 performance issue isn't very apparent at all, whereas it's immediately apparent after ~10ish pushes on device.

@satya164 My apologies I can't seem to get the chrome debugger to connect to either my genymotion or actual device for android (it works on iOS) on either 0.22.2 and 0.23-rc1 so I cannot provide this!

Perhaps we could try to reduce the number of scenes rendered.

renderScene(props) {
  const offset = props.scene.index - props.navigationState.index;
  if (Math.abs(offset) > 3) {
    // The scene is far way from the focused scene, don't render it.
    return null;
  }

  return <NavigationCard {...props} renderScene={this.props.renderScene} />;
}

+1 after ~10-15 pushes and pops my app crashes.

I'll investigate this ASAP.

FYI, this issue was hitting me hard and I believe I've fixed it by changing this line here:

https://github.com/facebook/react-native/commit/28649b8cf0a58fe4f32016a7ce6de1e35b349d2b#diff-04c37ff76f85aea35e8475617539b75fL251

and here

https://github.com/facebook/react-native/commit/28649b8cf0a58fe4f32016a7ce6de1e35b349d2b#diff-04c37ff76f85aea35e8475617539b75fL156

There was a bug where the animated values weren't actually being unsubscribed to:

this._layoutListeners.forEach(subscription => subscription.remove);

should become

this._layoutListeners.forEach(subscription => subscription.remove());

This is fixed in master.... but I'm not sure if it affects 0.23 or not. Also, others have mentioned that there is still a problem on devices even in 0.23. There may be more issues that are causing this, but this was definitely a big part of it.

cc @hedgerwang @ericvicenti

It looks from this discussion like this issue is fixed so I am going to close this issue.

Was this page helpful?
0 / 5 - 0 ratings