facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
119.38k stars 24.36k forks source link

[android][newarch] performance / requestAnimationFrame behavior changes #47888

Open lovegaoshi opened 17 hours ago

lovegaoshi commented 17 hours ago

Description

I've noticed requestAnimationFrame might behave differently between the old arch (snack expo 51) and the new arch (snack expo 52). in the provided snack, requestAnimationFrame is used to count the number of frames per second, which counts 60fps in old, 30fps in new.

on a real S21, a similar fps counter repo (repo init with expo cli https://github.com/lovegaoshi/rnbug-newarch-perf ) shows ~40-50 fps in new, ~70-100 fps in old. it all seems like requestAnimationFrame has only half the frame count in new vs old.

I'm investigating about the newarch performance @ https://github.com/facebook/react-native/issues/47490, theres the off chance this is due to the new arch's performance. worth noting the new arch's animation is visibly choppier than the old arch too.

Steps to reproduce

look at snack's top left corner

React Native Version

0.76.3

Affected Platforms

Runtime - Android

Areas

Fabric - The New Renderer, TurboModule - The New Native Module System

Output of npx react-native info

snack/expo 52

Stacktrace or Logs

n/a

Reproducer

https://snack.expo.dev/@lovegaoshi/thrilled-indigo-pizza

Screenshots and Videos

No response

react-native-bot commented 17 hours ago

[!WARNING] Could not parse version: We could not find or parse the version number of React Native in your issue report. Please use the template, and report your version including major, minor, and patch numbers - e.g. 0.76.2.

react-native-bot commented 17 hours ago

[!WARNING] Could not parse version: We could not find or parse the version number of React Native in your issue report. Please use the template, and report your version including major, minor, and patch numbers - e.g. 0.76.2.

lovegaoshi commented 16 hours ago

after forcing 60fps on s21 the choppiness is very easy to identify. and the nrw arch is also capped at 30, while old is at 60.

brentvatne commented 14 hours ago

you're not actually logging the fps as per rAF here, that would be done with something like this:

import { StyleSheet, Text, View } from 'react-native';

const start = performance.now();
let frames = 0;

const logFps = () => {
  frames++;
  console.log(frames / (performance.now() - start) * 1000);
  requestAnimationFrame(logFps);
}

logFps();

export default function App() {
  return (
    <View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
      <Text>look at the console</Text>
    </View>
  );
}

if you run this, you will notice that you get the fps that you expect.

what you are observing is likely the effect of using batched updates with the new react scheduler. state updates inside of timers are automatically batched. read more about batched state updates.

in your example, you put setState inside of a timeout inside of a requestAnimationFrame:

image

you shouldn't depend on setState executing synchronously here. maybe @rickhanlonii or @cipolleschi can explain how to flush state updates synchronously, since flushSync doesn't appear to be exposed by react-native

brentvatne commented 14 hours ago

if you remove the setTimeout(() => {}, 0) wrapping the above code from your example, it is once again 60fps.

it seems like what is desirable here is to perhaps document better some of the behavior changes in the scheduler. but i'm not convinced that this is necessarily a regression

lovegaoshi commented 13 hours ago

hi! appreciate the insight. that explanation makes sense. however i squinted my eyes and think the performance degradation is still present, or there is a migration guide i missed.

the repo is https://github.com/lovegaoshi/rnbug-newarch-perf. its a expo cli init project and the motionlayout animation built with reanimated. the video is recorded on a s21 60fps mode. first is old, second is new.

https://github.com/user-attachments/assets/ea471ce8-8360-44fa-a8a7-6713a894c665

brentvatne commented 13 hours ago

@lovegaoshi - it seems like that is a better issue to post to react-native-reanimated, no?

lovegaoshi commented 13 hours ago

actually i dont believe it's reanimated, as i did have smooth animation using the exact same implementation. the video below is the linked repo just one commit previous, also new arch; alas its smooth after putting to background then restored, somehow.

https://github.com/user-attachments/assets/54aed81f-6d50-4923-976b-6cedd3a33eac

i have to admit i believed the choppiness was due to whatever the fps logic is literally cutting the frame rate by half, as it does look like it. though this still seems like a reproducible example to show the new arch performance.

brentvatne commented 13 hours ago

@lovegaoshi - that may be the case, but keep in mind that reanimated also has some different codepaths and interactions with react-native when using the new architecture. just because a problem is apparent when you enable the new architecture doesn't immediately mean there is an issue in the new arch itself, it could be in the way that a library is integrating with it. we don't have enough information at this point to say where it is but i've pinged the reanimated team to see if they can take a look