-
Notifications
You must be signed in to change notification settings - Fork 474
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
Severe performance decrease after 1.10.2 update #2904
Comments
Thank you for the nice standalone example this looks very interesting, I'm gonna investigate this with the highest priority. You can use I'm really interested to see what might be happening there. |
Thanks! Let me know if you need any additional details. I also noticed that when I install the app in release mode on my android phone, performance is significantly better, but still lower than the previous version. |
Thank you again for the compelling example. Here I rewrote your example in a way that beats the performance of even the old canvas: https://gist.github.com/wcandillon/f315c9c9ec800c8302b33421700085da (no chance to have a frame drop here, I measured ~100x faster). That being said, this example, written the way you did, shouldn't be so slow. In its current form, this new reconciler is completely dependent on Hermes/JSI performance. We did a lot of benchmarking and an example like the one you provided completely escaped us (because indeed we would write it as in the gist I provided above). What we would benchmark instead is an example like the one you provided but where each tile is animated. At that scale, the user would have to engage as many reanimated derived values as tiles and therefore the JS overhead Skia would add on each frame would become smaller in comparison to the consumed frame budget (not negligible but definitely smaller). What we immediately did is to write a C++ version to remove JS out of the equation, but because we need to access the same value from Reanimated, we still need to use the JSI API (to execute sharedValue.value essentially). We have a prototype of this and it's definitely faster but not nearly as fast as we would have hoped. |
@wcandillon I wasn't aware of the drawAsPicture method, that's pretty cool. I wish I could use it, its just that in my actual app code (which is not that far from this example), the text on the tiles needs to change. Thank you for the in-depth explanation, I will be using the older implementation until you're able to fix the issue, no worries. I'm also looking for a way to render rectangles conditionally based on a sharedValue, and it seems like you might be able to bridge that gap aswell with the c++ API. I'm currently using runOnJS when a gesture ends instead of updating the values properly every frame. So in short, looking forward to it! |
Description
I'm seeing severe performance decrease, 10 to 20 fps drop when using the newest version of react-native-skia. I saw that a new reconciler was introduced and thought I might try to upgrade, but will roll back for now. Still wanted to report it.
I also upgraded react-native-reanimated and react-native-gesture-handler. These are on versions 3.16.7 and 2.22.0.
My canvas is very simple, a full screen gesture handler with around 100 rectangles in a group. The group can be panned and scaled. I was getting 60+ fps on version 1.8.1.
React Native Skia Version
1.10.2
React Native Version
0.76.5
Using New Architecture
Steps to Reproduce
Run the following code.
Snack, Code Example, Screenshot, or Link to Repository
The text was updated successfully, but these errors were encountered: