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

Firebase is keep crashing because of OOM on mobile Safari #6118

Closed
ocavue opened this issue Apr 4, 2022 · 39 comments · Fixed by #7569 or #7771
Closed

Firebase is keep crashing because of OOM on mobile Safari #6118

ocavue opened this issue Apr 4, 2022 · 39 comments · Fixed by #7569 or #7771

Comments

@ocavue
Copy link
Contributor

ocavue commented Apr 4, 2022

[REQUIRED] Describe your environment

  • Operating System version: iPhone iOS 15.4
  • Browser version: Safari 15
  • Firebase SDK version: 9.6.10
  • Firebase Product: Firestore

[REQUIRED] Describe the problem

We found that Firebase is keep crashing on mobile Safari because of out of memory. It seems that firebase is using much much more memory than expected.

We've created a minimal replication of this problem.

Source code: https://github.com/team-reflect/firebase-memory-issue

Online demo: https://firebase-memory-issue-2022-04.vercel.app/

In this replication, each document contains about 39KB of data (with a string length of about 39000). On my iPhone Xr (with 3GB of memory), this webpage will crash after loading about 750 documents, which is about 30MB of raw data in total. The browser memory usage is about 1.6GB right before the crashing.

Steps to reproduce:

Open the link above with iOS Safari. Click the click me to load 2000 documents button. Wait for the browser to load data and crash.

Connect the iPhone with a Mac then you can see the memory usage of the mobile Safari. You can see the memory usage of my iPhone Safari in the repo link above.

Relevant Code:

Check the source code link above.

@jbalidiong jbalidiong added the v9 label Apr 4, 2022
@wu-hui wu-hui self-assigned this Apr 5, 2022
@wu-hui
Copy link
Contributor

wu-hui commented Apr 7, 2022

Thanks for report this and an awesome reproduction.

I can confirm our memory usage is not ideal..I am not sure how to resolve this yet.

I want to ask if you have tried enabling persistence and if that changes the memory usage?

@maccman
Copy link

maccman commented Apr 7, 2022

@wu-hui I work with @ocavue.

Yes, we have enabled persistence. And no, it doesn't help reduce memory usage.

@maccman
Copy link

maccman commented Apr 14, 2022

Just following up here - we've been having some pretty bad production issues due to this memory bug.

@wu-hui
Copy link
Contributor

wu-hui commented Apr 14, 2022

Thanks.

Just to let you know, it will probably take some time for us to come up with some way to mitigate this, it probably requires some significant changes. In the meanwhile, if it is possible, you can try to limit the number of documents you read from the client.

@maccman
Copy link

maccman commented Apr 19, 2022

Thank you. In the meantime we are going to try and port the app to use Firebase Swift.

@wu-hui
Copy link
Contributor

wu-hui commented May 9, 2022

After looking into your reproduction a bit closer, it seems the Javascript Heap memory usage is not the merit, somehow "page" uses most of the memory in your reproduction, which is memory used by DOM, system memory, etc.

This is strange because the UI has minimum elements. Do you only observe this with Safari iOS, and other environments like Chrome/Firestore on Android do not have this issue?

I am also curious how Firebase Swift can help, do you mean you will be doing a native app instead of the web app?

@vojto
Copy link

vojto commented May 9, 2022

Hello, I'm working with @ocavue and @maccman.

After looking into your reproduction a bit closer, it seems the Javascript Heap memory usage is not the merit, somehow "page" uses most of the memory in your reproduction, which is memory used by DOM, system memory, etc.

This is strange because the UI has minimum elements. Do you only observe this with Safari iOS, and other environments like Chrome/Firestore on Android do not have this issue?

Yes, we've only seen it crashing on Safari/iOS.

I am also curious how Firebase Swift can help, do you mean you will be doing a native app instead of the web app?

Well, the cleanest solution would be to use a Capacitor plugin for Firestore.

  • The best library right now for Capacitor/Firebase integration is this one from @robingenz. It doesn't support Firestore yet, but looks like it's planned.
  • Then there's an older Cordova plugin. Haven't looked into this much.
  • In React Native land, there's a library. Porting that to Capacitor is probably out of the question.

Using one of these solutions would mean that any call such as ref('foo').getAll() would be sent to native layer, and Swift code would do the loading/caching, and then call your JavaScript with the result.

Another idea I have is to keep everything same as before, executed on web layer. Only the problematic data loads would be rewritten in Swift and wrapped in Capacitor plugin. The deal breaker is that you're either logging in to web layer, or native layer, can't log in to both.

@wu-hui
Copy link
Contributor

wu-hui commented May 9, 2022

Do you mind help us do a profile on Chrome or Firefox on other platforms, to see if you observe high memory usage as well? They might be using a lot memory as well, but managed not to crash. If their memory usage looks OK, we might need to report this to Apple as well.

I see what you are trying to achieve here, which makes it more important to verify if the issue is with iOS Safari's "page" memory usage. You might run into this again.

@ocavue
Copy link
Contributor Author

ocavue commented May 18, 2022

I did a profile on Chrome (v101.0.4951.67) and Firefox (v100.0.1) on Windows 11. I loaded 2000 records on our demo page.

Seems that both Chrome and Firefox use reasonable memory:

  • Chrome: Maximum memory usage is 293 MB
  • Firefox: Maximum memory usage is 631.46 MB

Let me know if there is anything else I can do.

@xar
Copy link

xar commented May 24, 2022

Isn't this related to - #4416 ? We have also experienced high memory usage when binding multiple collections sometimes It goest up to 1GB and after a while GC comes and memory goes back down to more reasonable level...

@wu-hui
Copy link
Contributor

wu-hui commented May 24, 2022

OK, I think at least we know this is an issue with Safari only. I'll do some research to see if there are similar issues reported before.

@maccman
Copy link

maccman commented Jun 9, 2022

Do you have any updates here @wu-hui ?

@yolpsoftware
Copy link

This also affects the NPM Firebase client when running it on a React Native app on an iOS device. Seems the default Javascript Engine of React Native on iOS is Safari. See here for a repro.

So the kinda obvious solution is to switch the React Native app to Hermes, the new Javascript Engine. I tested this and it works fine. No more gigabytes of memory just to load a few dozens of megabytes 😃

@yolpsoftware
Copy link

Just to let you know, it will probably take some time for us to come up with some way to mitigate this, it probably requires some significant changes. In the meanwhile, if it is possible, you can try to limit the number of documents you read from the client.

Curious to know how a browser-specific memory leak can require significant changes to fix.

@wu-hui
Copy link
Contributor

wu-hui commented Jun 17, 2022

Hi @maccman

Sorry for the late response. Unfortunately our own research found no clear way how we can fix this from the SDK side. I'd encourage you to file a tick to Safari instead. You can back link it here.

In the meanwhile, looks like @yolpsoftware has provided a viable workaround that is worth trying.

Hi @yolpsoftware

Thanks for sharing your solution. I do not understand your last comment, are you suggesting there is a potential easy fix we should explore? If yes, I'd like to hear more from you.

@vojto
Copy link

vojto commented Jun 17, 2022

It sounds like @yolpsoftware suggestion applies only to React Native.

I'm not clear on why you would even need that - wouldn't you just use react-native-firebase instead of the web client?

Seems like our best bet is building a Capacitor plugin to move Firestore work to native layer, @robingenz is working on one.

@maccman
Copy link

maccman commented Jun 17, 2022

Sorry for the late response. Unfortunately our own research found no clear way how we can fix this from the SDK side. I'd encourage you to file a tick to Safari instead. You can back link it here.

Can you elaborate here please. We are going to need some more information if we're going to file a ticket with Safari. Where exactly is the memory issue?

@yolpsoftware
Copy link

Yes my workaround applies to React Native only. React Native on iOS uses the Safari Javascript Engine by default. This is like Safari without a DOM. If the Firebase client (react-native-firebase too) runs in that Javascript Engine, it is affected by this bug.

My workaround is to use Hermes instead of the Safari Javascript Engine.

In the web scenario, this workaround of course does not work. Users on Safari are affected by this bug until it is fixed.

@yolpsoftware
Copy link

yolpsoftware commented Jun 17, 2022

Thanks for sharing your solution. I do not understand your last comment, are you suggesting there is a potential easy fix we should explore? If yes, I'd like to hear more from you.

@wu-hui I don't know anything about the internals of the Firebase client. It just occurs to me that in most scenarios such a bug should be fixable without a deep rewrite. It is your code that allocates that memory and does not free it. Profile your code by noting step-by-step what memory it allocates, and for what that memory is needed. Somewhere, you will find that your code uses much more memory when run in Safari than it does when run in Chrome or Firefox. When you find that, ask yourself why it needs that memory on Safari and not in Chrome. Then look for options on how to avoid such big memory allocations (e.g. by splitting arrays into parts that can be GC'ed earlier, or by clearing some internal caches).

Sorry if it's not that simple. I just haven't yet seen a scenario where such a big memory leak is not more or less easily fixable, but maybe I am mistaken.

@wu-hui
Copy link
Contributor

wu-hui commented Jun 17, 2022

@vojto That would also work, it is using the iOS SDK in this case.

@maccman I cannot, sorry. We hold a bunch of objects in memory, and this is usually under 100MB for other browsers, but on Safari it is GBs. We did not do anything special with Safari. One way you could try when you file a ticket to Apple, is to modify the web site you had but loading documents from Firestore Bundles, and also disableNetwork(). This way you can demonstrate this is a safari issue, without them running up you Firestore cost.

@yolpsoftware Thanks. We did not allocate more memory on Safari than other browsers, it seems Safari simple allocate more memory for the same set of documents. The only place we handle Safari differently is around IndexedDB initialization (where Safari is also problematic). I have not tried to do step by step tracing, I doubt I'll find anything new but I can give it a try when I find time.

@maccman
Copy link

maccman commented Jun 18, 2022

@wu-hui

This basically makes Firebase unusable on iOS Safari, and heavily performance degraded on desktop Safari. Selecting a few hundred records from Firestore is not an uncommon use-case.

Are you just not going to support those platforms?

Surely there is a way to debug this and get to the bottom of it? Like @yolpsoftware was saying - figure out which objects are using a lot of memory and seeing if there's some optimizations to be made. We'd be happy to help.

As it stands we are stuck. If we open a vague ticket with Safari they are just going to ask for more details.

@yolpsoftware
Copy link

yolpsoftware commented Jun 18, 2022

@yolpsoftware Thanks. We did not allocate more memory on Safari than other browsers, it seems Safari simple allocate more memory for the same set of documents. The only place we handle Safari differently is around IndexedDB initialization (where Safari is also problematic). I have not tried to do step by step tracing, I doubt I'll find anything new but I can give it a try when I find time.

If I were you, I would try to create an isolated reproduction of the problem. The isolated reproduction should be composed of a few dozen lines of code, and this code should allocate some memory, such that it holds under 100MB for any browser except Safari, where it is GBs.

Create a separate branch in your repository. In that branch, create some code that loads the amount of data that we are talking about. Then, step by step remove everything in the Firebase SDK that is not needed for the data load. Commit after every step. Then, mock away the networking stuff (sockets, XHR requests etc.). More and more, this code does not have anything to do with Firebase, because you have removed so much. It is just some lines of code that allocate some data objects in memory. If the problem goes away after you have taken something away (Safari's memory usage no longer problematic), then you have taken away too much, you need to go back (that's why you need to commit often - every commit says "until here, we still have the problem").

Step by step simplify everything until you either

  • have found the bug in your code, because you see what line causes it and why
  • or you have an isolated reproduction of the problem that has nothing to do with Firebase, which the Safari people will be happy to accept as a bug report.

@tebuevd
Copy link

tebuevd commented Jun 21, 2022

Please fix this :)

@yolpsoftware
Copy link

@wu-hui I really would like to know whether we can consider Firebase to be something that works across all current platforms and devices.

If you say, well, we can't fix / isolate that Safari bug, it's too difficult, then Firebase won't be an option for future projects.

@wu-hui
Copy link
Contributor

wu-hui commented Jul 13, 2022

Hi @yolpsoftware

Please keep in mind we are just a small team supporting all platforms, we have other features to deliver and more than half of the team is quite new to the product, things won't happen right away.

That being said, we do have someone assigned to this to further isolate the issue now, we will update this thread when we have something.

@yolpsoftware
Copy link

That being said, we do have someone assigned to this to further isolate the issue now, we will update this thread when we have something.

@wu-hui thank you, that's all I wanted to hear 😉

@wu-hui
Copy link
Contributor

wu-hui commented Aug 16, 2022

Just to give an update:

Our investigation seems to lead to web-channel triggering this high memory usage on Safari. While we are working with web-channel to sort out the root cause, we noticed the memory usage is lower if you enable experimentalForceLongPolling, to the point that is somewhat "reasonable".

It is obviously just a work-around, but I'd still like to share with you, hopefully this can alleviate your issues while we are trying to fix the root cause.

@maccman
Copy link

maccman commented Aug 18, 2022

Just to give an update:

Our investigation seems to lead to web-channel triggering this high memory usage on Safari. While we are working with web-channel to sort out the root cause, we noticed the memory usage is lower if you enable experimentalForceLongPolling, to the point that is somewhat "reasonable".

It is obviously just a work-around, but I'd still like to share with you, hopefully this can alleviate your issues while we are trying to fix the root cause.

Thank you so much - this is super helpful.

@IanPhilips
Copy link

IanPhilips commented Aug 31, 2022

Just to give an update:

Our investigation seems to lead to web-channel triggering this high memory usage on Safari. While we are working with web-channel to sort out the root cause, we noticed the memory usage is lower if you enable experimentalForceLongPolling, to the point that is somewhat "reasonable".

It is obviously just a work-around, but I'd still like to share with you, hopefully this can alleviate your issues while we are trying to fix the root cause.

This worked to fix our safari crashes, thank you for the work-around fix!! That being said, this issue should be surfaced during the installation documentation and its use highlighted! The plurality of our users are iphone users and accordingly this bug has caused a huge loss in UX.

I posted my results here: #1674 (comment)

@jomiplaz
Copy link

jomiplaz commented Sep 6, 2022

Same issue

@MedaiP90
Copy link

MedaiP90 commented Jul 24, 2023

Any update on this?
Setting experimentalForceLongPolling to true seems not working with @angular/fire 7.6.1 + firebase 9.22.1 (in a Ionic Capacitor project)

@sampajano
Copy link
Contributor

FYI, we were able to reproduce the issue and root cause the problem.. One of the mistake in Firestore's Fetch config has caused 10x+ memory been unnecessarily allocated on the heap, causing the crash (even though the memory will eventually be GC'ed)..

I'll work with the Firestore team (w/ @wu-hui et. al) to apply the fix and make a release as soon as possible..

Thanks for the patience here in the mean time!

@vojto
Copy link

vojto commented Aug 14, 2023

I'll work with the Firestore team (w/ @wu-hui et. al) to apply the fix and make a release as soon as possible..

We are very excited about this. Please keep us posted!

@wu-hui
Copy link
Contributor

wu-hui commented Aug 18, 2023

Sorry everyone for the long time it took to resolve this, and many many thanks to @sampajano for your effort on chasing down the rabbit hole!

This fix should be in the next SDK release, please test it and feel free to keep commenting on this issue if it did not work for you.

@maccman
Copy link

maccman commented Aug 22, 2023

Sorry everyone for the long time it took to resolve this, and many many thanks to @sampajano for your effort on chasing down the rabbit hole!

This fix should be in the next SDK release, please test it and feel free to keep commenting on this issue if it did not work for you.

Any idea when this is going to be released? We’d love to get testing.

@maccman
Copy link

maccman commented Aug 23, 2023

Looking good so far. Uses half the memory than the previous firebase version.

3A3FF8EE1B383608814F_0

@firebase firebase locked and limited conversation to collaborators Sep 18, 2023
@MarkDuckworth
Copy link
Contributor

Unfortunately, we had to roll back our fix for this and I must reopen it. The fix was attributed a more significant issue of hanging queries. This rollback (#7729) is included in v10.5.2. We currently have team members investigating the issues and I hope we can re-release a fix soon.

@wu-hui
Copy link
Contributor

wu-hui commented Nov 21, 2023

Unfortunately #7771 did not fix the memory issue completely, we are actively looking into this. Reopen this again..

@wu-hui wu-hui reopened this Nov 21, 2023
@wu-hui
Copy link
Contributor

wu-hui commented Nov 29, 2023

Things should be resolved in the latest 10.7.0 release. Sorry for the back-and-forth, and thanks for your patience!

@wu-hui wu-hui closed this as completed Nov 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.