-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[android][newarch] performance / requestAnimationFrame behavior changes #47888
Comments
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. |
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. |
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. |
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: you shouldn't depend on setState executing synchronously here. maybe @rickhanlonii or @cipolleschi can explain how to flush state updates synchronously, since |
if you remove the 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 |
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. Screen_Recording_20241121_153708_rnbugNew.mp4 |
@lovegaoshi - it seems like that is a better issue to post to react-native-reanimated, no? |
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. Screen_Recording_20241119_060028_rnbug.mp4i 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. |
@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 |
Hi @lovegaoshi. I tested this using the Android Studio Profiler on a Pixel 4a and got a comparable performance between architectures (with the New one being better). However on a lower-end device (Oppo A16) the experience was better (in Debug) on the Old Arch. I don't think there is a regression here that can be pinpointed to a singular change. I tried to figure out whether reanimated is doing something out of ordinary here (cc @brentvatne), but I didn't really find anything significant. In reanimated we use the RN api for updating the |
@lovegaoshi for a more public note on this, see @rubennorte's comment here. The implementation of react-native/packages/react-native/ReactCommon/react/runtime/TimerManager.cpp Lines 442 to 444 in 1a9780f
What this means is React Native doesn't called these at framerate intervals (but as part of the "regular" event loop), so you can't use it to measure frame rate. |
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 @ #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
Stacktrace or Logs
Reproducer
https://snack.expo.dev/@lovegaoshi/thrilled-indigo-pizza
Screenshots and Videos
No response
The text was updated successfully, but these errors were encountered: