fix(ios): avoid reallocating views on RCTDevLoadingView showMessage calls

[shirakaba]

Summary:

I'm upstreaming a change I've made to RCTDevLoadingView for the sake of react-native-macos, as I believe react-native could benefit from the same change.

As it regards the dev loading view, this issue of course affects only debug builds. The issue is that, each time a Metro progress update triggers the showMessage:withColor:withBackgroundColor: method on RCTDevLoadingView, we end up allocating a new UIWindow (iOS) or NSWindow (macOS), rather than reusing the existing window stored on self->_window from any previous progress updates. These unnecessary allocations are particularly expensive on macOS, as I show below.

Demo of the issue on macOS

On react-native-macos, the impact of this issue is dramatic (so much so that the Microsoft Office team disable RCTDevLoadingView in their Mac app). On the first connection to Metro (as first-time connections can produce nearly 100 progress updates), we end up allocating nearly 100 NSWindows. Allocating NSWindows is so expensive that it blocks the UI thread and adds 30 seconds to app launch (see 00:40 -> 01:15 of the below video clip).

What's more, as we can see in the view debugger, these NSWindows never get released, so they remain in the view graph and just leak memory (see 01:15 -> 01:30 of the below video clip).

(This clip is from 1:15:00 of a livestream where I dug into this problem – sorry for the poor quality clips, that's the best the servers allow me to download)

mwe_clip.mp4

Demo of the issue on iOS

The problem is, fortunately, unnoticeable on iOS (which is perhaps why it was overlooked when ported to macOS). I ran the same test on [email protected] and found that although React Native iOS does needlessly allocate many UIWindows, the allocations don't seem to delay app launch and no dangling windows show up in the view debugger. So it's possible that it's cheaper to allocate and dispose of windows in UIKit than in AppKit.

(This clip is from 3:03:45 of the livestream:)

output.mp4

But still, I feel it's good to align implementations and just good practice to avoid allocating memory needlessly, so I'd like to open this PR nonetheless. Heck, it might still be important for iOS on larger codebases that emit many more progress updates than the Hello World app demoed here.

Solution

Each of these views (the window, the container, and the label) need only be allocated once. Thereafter, they can be modified. This PR adds conditionals to lazily-allocate them, and reorders them in order of dependency (e.g. the label is the most nested, so needs to be handled first).

Changelog:

[IOS] [FIXED] - Avoid reallocating views on RCTDevLoadingView progress updates

Test Plan:

I am unable to run RNTester on the latest commit of the main branch; I've tried five different versions of Ruby, but the bundle install always fails. If someone could please guide me how to get RNTester, Ruby, and Bundler happy, I'd be happy to use that app to test it on main.

So my testing so far has been based on patching an existing [email protected] app live on stream. This version of React Native has the exact same bug as current main, so it should be representative.

[meta-codesync]

@sbuggay has imported this pull request. If you are a Meta employee, you can view this in D87465522.

[sbuggay]

Thanks for this! I've tested on RNTester internally and things seem to work fine.