[Discussion] refactor: optimize freeze state management in DelayedFreeze component

[zhouzh1]

Description

We're facing a performance issue which finally proved it can be mitigated by adjusting the logic of the DelayedFreeze component. The issue details are as follows.

1. Assuming that there are A, B, C three screens in the stack, A is in the bottom, C is in the top.
2. When we navigate from screen A to B, then to C and immediately after landing on the screen C, normally the screen A will get freezed.

However, the problem here is that,

1. The real freezing for the screen A will happen in an async callback scheduled by the setTimeout(..., 0)
2. In the right start of the screen C, we also have the corresponding code scheduling a state update to the screen A with a setTimeout(..., 0)
3. Based on the current setTimeout implementation from the react-native, for the above two setTimeout(..., 0) invocations, whose scheduled async callbacks are possible to get executed in the single tick, that means, we're possibly updating the state for the screen A and freezing the screen A nearly simultaneously, finally, maybe due to certain kind of race condition issue in the React Suspense component or in the React concurrent mode implementation, we found there would be a looks like cyclic JS code execution which causes the app to keep running with a high CPU consumption and the app screen loses responsiveness to the user interactions.

This issue might not happen every time when we do the same screen navigation, but the happening possiblity is not low in our app, after deep investigation we found the changes in this PR works well at most scenarios(at least we didn't encounter this performance issue anymore) and is able to avoid the similar issues from the root.

Let's back to the current implementation of delaying the freezing with the setTimeout(..., 0), I saw the comment says as follows.

// This component allows one more render before freezing the screen.
// Allows activityState to reach the native side and useIsFocused to work correctly.

However actually shoudn't the setTimeout(..., 0) allow multiple times of rendering instead of only one time before freezing? Conversely, seems that simply utilizing the useEffect to delay the freezing could really achieve the intention of allowing just one more time render before freezing the screen.

Finally, overall the principle behind this PR is seperating the screen freezing and the screen state updating into different ticks as much as possible, even though the real cause might be in the React framework itself intead of the react-native-screens, but if we could optimize it, then we should do.

Anyway, I am not 100% sure my changes won't introduce any new issues, so I'd like have a discussion with you to see if it makes sense. :)

Changes

1. Updated the DelayedFreeze component to make it accurately allow just more more render before screen freezing.

Screenshots / GIFs

Here you can add screenshots / GIFs documenting your change.

You can add before / after section if you're changing some behavior.

Before

After

-->

Test code and steps to reproduce

Checklist

• Included code example that can be used to test this change
• Updated TS types
• Updated documentation:
  • https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
  • https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
  • https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
  • https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
• Ensured that CI passes

[zhouzh1]

@kkafar @kmagiera Could you help take a look at this issue? Thanks.

[zhouzh1]

Hi @kligarski, I saw you added this setTimeout(..., 0) in the DelayedFreeze component before, could you please help take a look at this PR?

[kkafar]

Hey, thanks for raising the problem & preparing a PR. I'll add it to our backlog & will return here when it gets to top of it (hopefully soon).