-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Improved skipping commits #4833
Improved skipping commits #4833
Conversation
@michalmaka you're on fire! 💪 🔥 |
2064a35
to
79e6b16
Compare
|
@tomekzaw I've asked rn team to pick it on |
Hey folks, this was picked from rn team for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed internally, let's add #ifdefs for RN version not to change the behavior of 0.72.x since this PR requires things which were added in 0.72.4.
ae3bf02
to
61fd7ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR!
Summary
During investigation of race between RN and reanimated shadow tree committing I've noticed that if JS thread commits a new tree (triggering
pleaseSkipReanimatedCommit
) after startingcommit
from reanimated, the lattercommit
may cause JS commit failures.What's more, additional calls to
shouldReanimatedSkipCommit
result in avoiding CPU consuming copy operations on nodes and thus seems to work more performant.I added those checks just before each operation handling and just after full commit.
Chessboard example seemed to work a bit faster that without those changes. I used also profiler and in overall CPU spends less time in ShadowTree.
NOTICE⚠️
This change requires change in RN codebase. PR has been submitted facebook/react-native#38715
Test plan
Check performance of e.g. chessboard example.