-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[🐛] Firestore Get query 4x slower on Android than iOS #4270
Comments
I think there's already an issue for this. Please note that "80 documents" means effectively nothing as "documents" can be wildly different. If you want to do valid benchmarks and discuss performance then actual data in use needs to be specified precisely and timed across versions for comparison. To be completely clear: precise and accurate specification of exact data and versions is a strict requirement for performance discussion The related work in the area is here If I recall someone was trying to build a quick "data set generator" such that the data under test could be controlled, then tests could be run in a small test harness. That's the sort of thing needed in order to actually fix issues like this - reproducible test case, then comes the incremental improvement |
Hello Mike, Thanks for your answer! We were already following those two topics, however the solutions did not fix our issue. To be more precise about our request in order to reproduce : each document is composed of 9 strings & 1 object of 30 elements. We tried to downgrade to previous Firebase SDK versions (until the Firebase BoM 24.1.0 which was first used in RNFirebase V6), but it did not change anything to our issue. Thanks for your feedback! |
I think that there is a performance issue between v5 and v6, I just think we don't have the rig set up to profile it and fix it - an app.js that could generate a set of data on demand that usefully showed the problem and exercised it with timing displayed. It's not the biggest hurdle to get over but it is the first step. Then it's a process of profiling + investigating the slow points + fixing + repeat |
Hi Mike, Thanks for your answer, as requested, we tested with a simple reproducible GET function on 300 identical documents on iOS and Android, here are the steps we followed:
App.js:
testUser.json:
Here are our results: Android : ~7000 ms on 1st launch, then ~5000 ms I hope it will be easy enough for you to reproduce, and share with us any insights about our results (or yours if you get the same). Thanks a lot for your time! |
Fantastic! This should help a lot - let me collaborate with the Invertase folks to see what we can do with regard to a concerted performance tuning cycle. They're focus right now is substantially shifted to a separate work contract at the moment and I'm pretty busy myself but this has also been a long-standing issue and you've served it up nicely to reproduce! Just in case you're hungry to go through it yourself and not wait (always the fastest way to get things done, FWIW) the next step is to get set up in Android Studio with profiling enabled and either use the IDE tools to profile (they are pretty good!) or set up code timers etc to effectively do the same, and start doing the typical performance tuning "test and take timings, make small change, repeat" loop |
Thanks for your support and for bringing it to the team! Did you manage to reproduce it yourself, and, if so, did you notice a platform difference as we did? I tried to use Android Studio Profiler to debug and analyze this request, however I am not used to it, so I am not really sure about what I should be looking for, any clue about this? |
Invertase appears to be booked at the moment and I'm still booked myself as well unfortunately, I have not dropped it in my test harness yet to reproduce but having it as a self-contained App.js is beautiful so I'm reasonably sure I'll reproduce when I try. My hunch (could be wrong!) is that something inefficient is being down with regards to data going back and forth across the react-native native/JS bridge, and it's CPU bound. Those are separate assertions and each bears examining, the yes/no test on whether they are correct will be found doing this: https://developer.android.com/studio/profile/cpu-profiler |
Hi again, |
@BatRisbec thanks for the update. Sorry there was nothing conclusive, I guess I was hoping it was some obvious serialization issue on the JS/Native bridge, a shame nothing was obvious There are no updates though, this is the issue tracking it here and you can see where we are (you are the only one testing right now, unfortunately) and unless you have a specific tracking issue in the firebase-android-sdk library with a clean reproduction from them based on their quickstarts, there is most likely nothing there. It's possible - if this is a problem reproducible on a firebase-android-sdk firestore quickstart project - to create an issue there though and see official attention from the firebase team |
I'm facing with same issue. |
@lnminh58 there was discussion that for some people, downgrading the android BOM (and thus firestore version) to lower versions - or perhaps moving forward to bom 25.11.0) had an effect. I recommend a quick (likely just 3 rebuilds) bisect of versions to see if that impacts your case. |
As a side-note, FYI, we upgraded from @react-native-firebase/firestore 7.3.2 to 7.8.6 and Firestore's doc |
Hi @mikehardy, did you have by any chance the occasion to test our demo? Thanks for your time! |
Sorry I haven't had the time to test it @BatRisbec - I can say that I worked with @MujtabaFR to update the SDKs we rely on / use by default, and there is word that the new BoM (above 25.10.0 I think? we just bumped to 25.12.0) improves things. If you were not already overriding the BOM in your project, you might either try that or update to the new versions of react-native-firebase that are publishing as I type this and see if that helps you? |
Wow amazing, we tested the latest combo App / Firestore / BoM versions, and it seems to work! Thanks a lot for having kept us updated on the topic! |
Likewise thanks for letting me know your testing result to date - I wasn't sure how we were going to troubleshoot this coherently so I'm happy to hear a BoM upgrade could do it. I will preliminarily close this but as it does have a repro and other great info I'll stand by to reopen if there seems to be something going on in this module that the new BoM did not fix. Cheers @BatRisbec |
how did you do ? can you share the result ? |
@Thanh90 not sure on exactly what combination is now working (and what exactly "working" means for the original reporter) but the "BoM" is the android "bill of materials" - https://firebase.google.com/docs/android/learn-more#bom - and you may override it if you like, like so: https://rnfirebase.io/#android - however you may also see that we are well past the version mentioned here (25.something) by consulting our current BoM dependency:
|
@mikehardy I can confirm that there's no problem with v12.1.0. the slow caused by my processing after loading data from snapshot. loading data from snapshot is fast |
Hi there,
We recently migrated from RNFirebase V5 to V6 and everything went well except for Firestore where we are experiencing some slow requests ONLY on Android (4x slower).
Test to reproduce :
Promise.all([ function1.get(), function2.get(), function3.get(), ]) .then(snaps => { console.log(snaps) })
Each function reads and gets ~80 documents. On iOS we get the log in about 2 seconds, when we need at least 8 seconds on Android (tested both in emulators and real devices).
The request reads and gets exactly the same documents.
We are using Firebase Android 25.5.0 & Firebase iOS 6.28.1. We used to have Firebase Android 25.7.0, but it was even slower than now...
Would you have any idea to explain such a huge difference ?
Thanks for your time !
Project Files
"@react-native-firebase/app": "^8.3.1",
"@react-native-firebase/firestore": "^7.5.3",
Have you converted to AndroidX? Yes
android/gradle.settings
jetifier=true
for Android compatibility?jetifier
for react-native compatibility?android/build.gradle
:Environment
npmPackages:
react: ^16.8.6 => 16.9.0
react-native: 0.60.6 => 0.60.6
npmGlobalPackages:
react-native-cli: 2.0.1
react-native-git-upgrade: 0.2.7
react-native-rename: 2.2.2
The text was updated successfully, but these errors were encountered: