Skip to content

Commit 74bdab8

Browse files
Gaspard Viotfacebook-github-bot
authored andcommitted
Reduce memory allocations when computing accessibilityLabel (#44605)
Summary: While investigating the root cause of app hanging on older devices in Instruments, I noticed that the heaviest stack trace was pointing to `RCTRecursiveAccessibilityLabel` in RCTView.m. <details> <summary>Heaviest stack trace in Instruments</summary> <img width="473" alt="Screenshot 2024-05-17 at 4 22 48 PM" src="https://github.com/facebook/react-native/assets/849905/fab8ed01-7a2f-4113-b2ca-04e76f25cd9d"> </details> The profiling was done on an iPad (5th generation) running iOS 16.7.4. The app is text heavy which makes the issue more visible than in RNTester for instance. ### Before <img width="854" alt="Screenshot 2024-05-17 at 4 19 46 PM" src="https://github.com/facebook/react-native/assets/849905/5e3cc7ad-299c-4814-ab4a-031c0e677b12"> It turns out that `[NSMutableString stringWithString:@""]` is initialized in every call of the recursion even though most of the time it's only used to check the length at the end and return `nil`. My change only initialize the mutable string if it's going to be used. I applied the same logic to the equivalent Fabric component. It's a small change that improved the accessibility label generation by 60ms in my case. ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [IOS] [CHANGED] - Reduce memory allocations when computing accessibilityLabel Pull Request resolved: #44605 Test Plan: Running the same measurements after the change, computing the accessibility label is not the heaviest stack trace anymore. And the line by line tracing shows that `[NSMutableString stringWithString:@""]` impact has been significantly reduced. ### After <img width="769" alt="Screenshot 2024-05-17 at 4 53 04 PM" src="https://github.com/facebook/react-native/assets/849905/1ad638ac-ba7e-4dca-ac77-10df5d2dad49"> I have been using this change in production thanks to a patch-package and it effectively improved the performances when navigating between screens. I also tested in RNTester with and without Fabric. For both architectures, I made sure the return value of `RCTRecursiveAccessibilityLabel`. Interestingly, when there is no label, the Fabric implementation returns an empty string while the `RCTView.m` returns `nil`. I'm open to align both implementations to return `nil` if you believe there is no underlying reason requiring the Fabric implementation to return an empty string. Reviewed By: cipolleschi Differential Revision: D67620818 Pulled By: javache fbshipit-source-id: 1a6937075a5ff5a9ad03fbbf910d64b3884c0fe0
1 parent 6d2b616 commit 74bdab8

2 files changed

Lines changed: 11 additions & 3 deletions

File tree

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1123,13 +1123,17 @@ - (NSObject *)accessibilityElement
11231123

11241124
static NSString *RCTRecursiveAccessibilityLabel(UIView *view)
11251125
{
1126-
NSMutableString *result = [NSMutableString stringWithString:@""];
1126+
// Result string is initialized lazily to prevent useless but costly allocations.
1127+
NSMutableString *result = nil;
11271128
for (UIView *subview in view.subviews) {
11281129
NSString *label = subview.accessibilityLabel;
11291130
if (!label) {
11301131
label = RCTRecursiveAccessibilityLabel(subview);
11311132
}
11321133
if (label && label.length > 0) {
1134+
if (result == nil) {
1135+
result = [NSMutableString string];
1136+
}
11331137
if (result.length > 0) {
11341138
[result appendString:@" "];
11351139
}

packages/react-native/React/Views/RCTView.m

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,20 +89,24 @@ - (UIView *)react_findClipView
8989

9090
static NSString *RCTRecursiveAccessibilityLabel(UIView *view)
9191
{
92-
NSMutableString *str = [NSMutableString stringWithString:@""];
92+
// Result string is initialized lazily to prevent useless but costly allocations.
93+
NSMutableString *str = nil;
9394
for (UIView *subview in view.subviews) {
9495
NSString *label = subview.accessibilityLabel;
9596
if (!label) {
9697
label = RCTRecursiveAccessibilityLabel(subview);
9798
}
9899
if (label && label.length > 0) {
100+
if (str == nil) {
101+
str = [NSMutableString string];
102+
}
99103
if (str.length > 0) {
100104
[str appendString:@" "];
101105
}
102106
[str appendString:label];
103107
}
104108
}
105-
return str.length == 0 ? nil : str;
109+
return str;
106110
}
107111

108112
@implementation RCTView {

0 commit comments

Comments
 (0)