Skip to content

Commit 55a5b6b

Browse files
shirakabameta-codesync[bot]
authored andcommitted
fix(ios): avoid reallocating views on RCTDevLoadingView showMessage calls (#54608)
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 `NSWindow`s. Allocating `NSWindow`s 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 `NSWindow`s **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](https://www.youtube.com/live/amRWVbfbknM?si=KmDBXjrQXdxnmf1E&t=4500) where I dug into this problem – sorry for the poor quality clips, that's the best the servers allow me to download) https://github.com/user-attachments/assets/65bb7c9b-dc18-4e54-8369-d0c611c59439 ### 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 `UIWindow`s, the allocations don't seem to delay app launch and no danglings 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:) https://github.com/user-attachments/assets/a0f841f4-d55b-403d-8abb-5af56af5758d 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 Pull Request resolved: #54608 Test Plan: I am unable to run `RNTester` on the latest commit of the `main` branch; I've tried [five different versions](https://x.com/birch_js/status/1991129150728642679?s=20) 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. Reviewed By: shwanton Differential Revision: D87465522 Pulled By: sbuggay fbshipit-source-id: f5f270082f1a9c038760054143a817885131ef85
1 parent d553c31 commit 55a5b6b

File tree

1 file changed

+54
-46
lines changed

1 file changed

+54
-46
lines changed

packages/react-native/React/CoreModules/RCTDevLoadingView.mm

Lines changed: 54 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -116,58 +116,66 @@ - (void)showMessage:(NSString *)message
116116

117117
self->_showDate = [NSDate date];
118118

119-
UIWindow *mainWindow = RCTKeyWindow();
120-
self->_window = [[UIWindow alloc] initWithWindowScene:mainWindow.windowScene];
121-
self->_window.windowLevel = UIWindowLevelStatusBar + 1;
122-
self->_window.rootViewController = [UIViewController new];
123-
124-
self->_container = [[UIView alloc] init];
125-
self->_container.backgroundColor = backgroundColor;
126-
self->_container.translatesAutoresizingMaskIntoConstraints = NO;
127-
UITapGestureRecognizer *tapGesture = [[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(hide)];
128-
[self->_container addGestureRecognizer:tapGesture];
129-
self->_container.userInteractionEnabled = YES;
130-
131-
if (dismissButton) {
132-
CGFloat hue = 0.0;
133-
CGFloat saturation = 0.0;
134-
CGFloat brightness = 0.0;
135-
CGFloat alpha = 0.0;
136-
[backgroundColor getHue:&hue saturation:&saturation brightness:&brightness alpha:&alpha];
137-
UIColor *darkerBackground = [UIColor colorWithHue:hue
138-
saturation:saturation
139-
brightness:brightness * 0.7
140-
alpha:1.0];
141-
142-
UIButtonConfiguration *buttonConfig = [UIButtonConfiguration plainButtonConfiguration];
143-
buttonConfig.attributedTitle = [[NSAttributedString alloc]
144-
initWithString:@"Dismiss ✕"
145-
attributes:@{NSFontAttributeName : [UIFont systemFontOfSize:11.0 weight:UIFontWeightRegular]}];
146-
buttonConfig.contentInsets = NSDirectionalEdgeInsetsMake(6, 12, 6, 12);
147-
buttonConfig.background.backgroundColor = darkerBackground;
148-
buttonConfig.background.cornerRadius = 10;
149-
buttonConfig.baseForegroundColor = color;
150-
151-
UIAction *dismissAction = [UIAction actionWithHandler:^(__kindof UIAction *_Nonnull action) {
152-
[self hide];
153-
}];
154-
self->_dismissButton = [UIButton buttonWithConfiguration:buttonConfig primaryAction:dismissAction];
155-
self->_dismissButton.translatesAutoresizingMaskIntoConstraints = NO;
119+
if (self->_label == nullptr) {
120+
self->_label = [[UILabel alloc] init];
121+
self->_label.translatesAutoresizingMaskIntoConstraints = NO;
122+
self->_label.font = [UIFont monospacedDigitSystemFontOfSize:12.0 weight:UIFontWeightRegular];
123+
self->_label.textAlignment = NSTextAlignmentCenter;
156124
}
157-
158-
self->_label = [[UILabel alloc] init];
159-
self->_label.translatesAutoresizingMaskIntoConstraints = NO;
160-
self->_label.font = [UIFont monospacedDigitSystemFontOfSize:12.0 weight:UIFontWeightRegular];
161-
self->_label.textAlignment = NSTextAlignmentCenter;
162125
self->_label.textColor = color;
163126
self->_label.text = message;
164127
self->_label.numberOfLines = 0;
165128

166-
[self->_window.rootViewController.view addSubview:self->_container];
167-
if (dismissButton) {
168-
[self->_container addSubview:self->_dismissButton];
129+
if (self->_container == nullptr) {
130+
self->_container = [[UIView alloc] init];
131+
self->_container.translatesAutoresizingMaskIntoConstraints = NO;
132+
UITapGestureRecognizer *tapGesture = [[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(hide)];
133+
[self->_container addGestureRecognizer:tapGesture];
134+
self->_container.userInteractionEnabled = YES;
135+
136+
if (dismissButton) {
137+
CGFloat hue = 0.0;
138+
CGFloat saturation = 0.0;
139+
CGFloat brightness = 0.0;
140+
CGFloat alpha = 0.0;
141+
[backgroundColor getHue:&hue saturation:&saturation brightness:&brightness alpha:&alpha];
142+
UIColor *darkerBackground = [UIColor colorWithHue:hue
143+
saturation:saturation
144+
brightness:brightness * 0.7
145+
alpha:1.0];
146+
147+
UIButtonConfiguration *buttonConfig = [UIButtonConfiguration plainButtonConfiguration];
148+
buttonConfig.attributedTitle = [[NSAttributedString alloc]
149+
initWithString:@"Dismiss ✕"
150+
attributes:@{NSFontAttributeName : [UIFont systemFontOfSize:11.0 weight:UIFontWeightRegular]}];
151+
buttonConfig.contentInsets = NSDirectionalEdgeInsetsMake(6, 12, 6, 12);
152+
buttonConfig.background.backgroundColor = darkerBackground;
153+
buttonConfig.background.cornerRadius = 10;
154+
buttonConfig.baseForegroundColor = color;
155+
156+
UIAction *dismissAction = [UIAction actionWithHandler:^(__kindof UIAction *_Nonnull action) {
157+
[self hide];
158+
}];
159+
self->_dismissButton = [UIButton buttonWithConfiguration:buttonConfig primaryAction:dismissAction];
160+
self->_dismissButton.translatesAutoresizingMaskIntoConstraints = NO;
161+
[self->_container addSubview:self->_dismissButton];
162+
}
163+
[self->_container addSubview:self->_label];
164+
}
165+
self->_container.backgroundColor = backgroundColor;
166+
167+
UIWindow *mainWindow = RCTKeyWindow();
168+
if (self->_window == nullptr) {
169+
UIWindowScene *windowScene = mainWindow.windowScene;
170+
if (windowScene != nil) {
171+
self->_window = [[UIWindow alloc] initWithWindowScene:windowScene];
172+
} else {
173+
self->_window = [[UIWindow alloc] init];
174+
}
175+
self->_window.windowLevel = UIWindowLevelStatusBar + 1;
176+
self->_window.rootViewController = [UIViewController new];
177+
[self->_window.rootViewController.view addSubview:self->_container];
169178
}
170-
[self->_container addSubview:self->_label];
171179

172180
CGFloat topSafeAreaHeight = mainWindow.safeAreaInsets.top;
173181
self->_window.hidden = NO;

0 commit comments

Comments
 (0)