Reduce memory allocations when computing accessibilityLabel

[sparga]

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.

Heaviest stack trace in Instruments

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

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:

[IOS] [CHANGED] - Reduce memory allocations when computing accessibilityLabel

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

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.

[javache]

Thanks for contributing! I think getting the two implementations to match and returning nil when there is no label would be desired.

[sparga]

Sorry, it took me a while to address the feedback but I finally took the time to do it.

[facebook-github-bot]

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@javache merged this pull request in facebook/react-native@74bdab8bd8be2413734004145507c0688232053e.

[react-native-bot]

This pull request was successfully merged by @sparga in 74bdab8bd8be2413734004145507c0688232053e

^{When will my fix make it into a release? | How to file a pick request?}