Skip to content

Commit

Permalink
Reduce memory allocations when computing accessibilityLabel (#44605)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sparga authored and facebook-github-bot committed Dec 24, 2024
1 parent 6d2b616 commit 74bdab8
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1123,13 +1123,17 @@ - (NSObject *)accessibilityElement

static NSString *RCTRecursiveAccessibilityLabel(UIView *view)
{
NSMutableString *result = [NSMutableString stringWithString:@""];
// Result string is initialized lazily to prevent useless but costly allocations.
NSMutableString *result = nil;
for (UIView *subview in view.subviews) {
NSString *label = subview.accessibilityLabel;
if (!label) {
label = RCTRecursiveAccessibilityLabel(subview);
}
if (label && label.length > 0) {
if (result == nil) {
result = [NSMutableString string];
}
if (result.length > 0) {
[result appendString:@" "];
}
Expand Down
8 changes: 6 additions & 2 deletions packages/react-native/React/Views/RCTView.m
Original file line number Diff line number Diff line change
Expand Up @@ -89,20 +89,24 @@ - (UIView *)react_findClipView

static NSString *RCTRecursiveAccessibilityLabel(UIView *view)
{
NSMutableString *str = [NSMutableString stringWithString:@""];
// Result string is initialized lazily to prevent useless but costly allocations.
NSMutableString *str = nil;
for (UIView *subview in view.subviews) {
NSString *label = subview.accessibilityLabel;
if (!label) {
label = RCTRecursiveAccessibilityLabel(subview);
}
if (label && label.length > 0) {
if (str == nil) {
str = [NSMutableString string];
}
if (str.length > 0) {
[str appendString:@" "];
}
[str appendString:label];
}
}
return str.length == 0 ? nil : str;
return str;
}

@implementation RCTView {
Expand Down

0 comments on commit 74bdab8

Please sign in to comment.