Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scanning all Objective-C classes on +(void)load slows down app startup time #10

Open
naruaway opened this issue Jul 10, 2024 · 11 comments

Comments

@naruaway
Copy link

Hi, thanks for this nice library! We found one performance issue and we fixed it by ourselves with some workaround but I'll leave the description here.

When using ReactBridge, the app invokes this function on startup and when the app is big enough, this can take more than 500ms. In our case, the number of Objective-C classes was more than 50000. Since most of Objective-C classes are not relevant for ReactBridge, this is inefficient. I am curious whether we have alternative way to call _registerModule without scanning all the Objective-C classes and methods.

@naruaway naruaway changed the title Getting all Objective-C classes on +(void)load slows down app startup time Scanning all Objective-C classes on +(void)load slows down app startup time Jul 10, 2024
@ikhvorost
Copy link
Owner

Hi! Thanks for issue reporting!

Have you observed the problem on v1.1.0? Because I've moved the registering code from __attribute__((constructor)) func in v1.1.0 to +load method in v1.2.0 and it may cause delay.

@naruaway
Copy link
Author

Hi @ikhvorost , thanks for the quick reply! I forgot the exact library version but we noticed the slowdown for the version with __attribute__((constructor)). I think the issue should be the same for +load since the problem is the iteration of all the Objective-C classes.

FYI we just decided to use something like the following:

// Define this

private static let reactNativeRegisterModulesOnce: Void = {
  RCTRegisterModule(MyRnModuleABC.self)
  RCTRegisterModule(MyRnModuleXYZ.self)
}()

// And call the following just before RCTBridge initialization

_ = reactNativeRegisterModulesOnce

In this way module registration does not have additional overhead of iterating through all the Objective-C classes. The drawback is we need to maintain this list manually. For our use case this is totally fine but I think this is not good as a library.

I was thinking to abstract away this in Macro but maybe it's hard? It looks like there is no way to call something "very early" reliably in Swift. Lazy initialization (e.g. lazy) does not help since the library cannot assume the timing when RCTBridge gets initialized... If there is a way to generate C/Objective-C code from Swift macro, we can generate specific RCTRegisterModule calls though.

@ikhvorost
Copy link
Owner

Hi @naruaway!

The registering code takes about 15 ms with 34673 classes from my tests on a real iPhone. Don't know why but your code maybe have many classes with many methods so that they consume much time to process.

Anyway I'm considering optimising auto registration for the react module classes.

@naruaway
Copy link
Author

naruaway commented Jul 13, 2024

Thanks @ikhvorost! Actually for our case it was around 50000 classes so 15ms is much faster than what I measured. Also note that the inner loop count was around 90000.

And for my case, it was around 500ms. I am curious, how did you measure that? I ran xcrun xctrace record --device MYDEVICE_UDID_HERE --template 'App Launch' --time-limit 3s --launch MY_APP_NAME_HERE --output /path/to/output for 300 times per each variant (the one with this code (well, the one with __attribute__((constructor))) and the one without the Objective-C file) using a script with a real device to take 50 percentiles for the durations.

BTW I observed the original issue for my work and the app is not open source. Also for this work we are fine with RCTRegisterModule(MyRnModuleABC.self) since this does not have any additional overhead. However, I am personally (I mean, not related to work) very interested for the performance characteristic of this way of finding the target classes. Let me try to create a minimum reproducible for the benchmark when I have time. Stable benchmarking is really hard... actually I did the local benchmark again and the diff was then around 100ms. Still much larger than 15ms though.

@ikhvorost
Copy link
Owner

I just use for my code:

NSDate *date = NSDate.date;

// Hard work
...

NSLog(@"Timeout: %f", -date.timeIntervalSinceNow);

@naruaway
Copy link
Author

naruaway commented Jul 15, 2024

@ikhvorost thanks, I used -date.timeIntervalSinceNow with a fresh app and confirmed that with my own device (iPhone 13 mini), it was around 20 - 40ms for repeated executions.

When I deleted the app and launch it for the first time, it's around 300ms so I think the duration changes drastically depending on the "coldness" of the app. Anyway even 20ms - 40ms is kind of too much just for registering the classes.

Just note that for our app, we noticed the slowdown via Xcode Organizer and it was around 500ms for the 50th percentile. It should be also because our app is definitely larger than "empty app". So I think this slowdown is noticeable for real users for real world big enough apps

@naruaway
Copy link
Author

naruaway commented Jul 15, 2024

@ikhvorost I created a reproducible script for the benchmark with xctrace: https://github.com/naruaway-sandbox/2024-07-ios-iterate-all-objc-classes-perf

TL;DR; For my iPhone 13 mini with minimal "hello world" app, "from app start to static initialization" increased 37 ms, which is 40% increase and this is significant. I guess this slowdown might be non-linear for the app complexity increase.

Screenshot 2024-07-15 at 19 39 35

(I put the above image just for a quick reference: these images with more details are in https://github.com/naruaway-sandbox/2024-07-ios-iterate-all-objc-classes-perf)

@ikhvorost
Copy link
Owner

ikhvorost commented Jul 16, 2024

Hi @naruaway!

I've tested ReactBridge on my huge real project and got the same result: 53508 classes and ~60 ms registering delay.

The root cause of the issue is running code on the main thread and it blocks loading of an app. So I've moved execution of the code to the background thread and now it doesn't block the main one anymore.

You can try the changes with new version 1.2.1.

@naruaway
Copy link
Author

Thanks! Now I think in the real world with real devices, which can include legacy devices, the slowdown can be higher than experiments with our own (stable and new?) devices

Oh the solution looks interesting! But is it really 100% safe to call these Objective-C related functions in a background thread? I think for now we would keep using our own way (#10 (comment)) for simplicity / predictability 🙏 . Actually I think in some edge cases registration might not complete when the first RCTBridge gets loaded in this way. Comparing with-init and without-init in this trace, I think that issue could really happen.

Hmm it's perfect if there is a way to generate C / ObjC from Swift macro to automatically derive RCTRegisterModule calls...
But we merged #10 (comment) several days ago in our app and deployed to PROD today, and now I feel like this way is not so bad as I initially thought since:

  • When adding a new class, if someone forget adding RCTRegisterModule(MyRnModuleXYZ.self), manual test for the corresponding RN feature will immediately reveal the missing module
  • When removing MyRnModuleXYZ from Swift code, RCTRegisterModule(MyRnModuleXYZ.self) will cause compile error so we never forget cleaning up RCTRegisterModule calls

@ikhvorost
Copy link
Owner

First, the registering code under __attribute__((constructor)) is called after all classes are already loaded and before the main function starts. Also the code has enough time before React Native finish loading index.js and starts making bridges. Also RCTRegisterModule is a non blocking thread safe function and can be called from any thread.

But you are right the best case is calling it for a particular class directly (like ObjC macros do) on an app startup and now I'm probing other future approaches like @_section("section_name") etc.

@naruaway
Copy link
Author

naruaway commented Jul 17, 2024

Thanks for clarification! Good to know it's thread-safe 🙏

Also the code has enough time before React Native finish loading index.js and starts making bridges

I think my main concern is this. In our app, React Native is just a small piece of experiment and we do not want to take any risk to affect the app itself even for very rare edge case. I think there is no guarantee for the timing when the bridge creation finishes.

I'm probing other future approaches

Cool! Now thinking it might be reasonable to provide "manual registration mode" as well (people can vendor & patch like we did though) especially for users who do not need React Native on start up (with background thread it should not matter much for this case but there are various types of apps; maybe some apps have already optimized things a lot and they might be saturating all the CPU cores on start up.. )

BTW I was browsing through Expo related things on the internet and just found ReactBridge is mentioned in https://github.com/EvanBacon/more-reacty-native-demo/blob/main/README.md#other-cool-stuff 🚀 by Evan Bacon! And coincidentally, "Indeed Job Search" app is what I was helping the performance issue (not sure how he found and how he reverse-engineered our app). Our app is legacy & big app and React Native is only used for small part of UI as an experiment. ReactBridge is so nice since we can just use clean Swift! Thank you so much for this library ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants