firebase-js-sdk icon indicating copy to clipboard operation
firebase-js-sdk copied to clipboard

Firebase is keep crashing because of OOM on mobile Safari

Open ocavue opened this issue 3 years ago • 30 comments

[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.

ocavue avatar Apr 04 '22 11:04 ocavue

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?

wu-hui avatar Apr 07 '22 17:04 wu-hui

@wu-hui I work with @ocavue.

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

maccman avatar Apr 07 '22 20:04 maccman

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

maccman avatar Apr 14 '22 09:04 maccman

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.

wu-hui avatar Apr 14 '22 19:04 wu-hui

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

maccman avatar Apr 19 '22 13:04 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?

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

wu-hui avatar May 09 '22 14:05 wu-hui

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.

vojto avatar May 09 '22 15:05 vojto

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.

wu-hui avatar May 09 '22 15:05 wu-hui

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.

ocavue avatar May 18 '22 00:05 ocavue

Isn't this related to - https://github.com/firebase/firebase-js-sdk/issues/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...

xar avatar May 24 '22 16:05 xar

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.

wu-hui avatar May 24 '22 18:05 wu-hui

Do you have any updates here @wu-hui ?

maccman avatar Jun 09 '22 01:06 maccman

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 avatar Jun 16 '22 21:06 yolpsoftware

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.

yolpsoftware avatar Jun 16 '22 21:06 yolpsoftware

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.

wu-hui avatar Jun 17 '22 15:06 wu-hui

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.

vojto avatar Jun 17 '22 15:06 vojto

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?

maccman avatar Jun 17 '22 16:06 maccman

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 avatar Jun 17 '22 16:06 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.

@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.

yolpsoftware avatar Jun 17 '22 17:06 yolpsoftware

@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.

wu-hui avatar Jun 17 '22 17:06 wu-hui

@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.

maccman avatar Jun 18 '22 18:06 maccman

@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.

yolpsoftware avatar Jun 18 '22 20:06 yolpsoftware

Please fix this :)

tebuevd avatar Jun 21 '22 16:06 tebuevd

@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.

yolpsoftware avatar Jul 13 '22 14:07 yolpsoftware

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.

wu-hui avatar Jul 13 '22 15:07 wu-hui

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 😉

yolpsoftware avatar Jul 13 '22 15:07 yolpsoftware

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.

wu-hui avatar Aug 16 '22 18:08 wu-hui

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.

maccman avatar Aug 18 '22 03:08 maccman

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: https://github.com/firebase/firebase-js-sdk/issues/1674#issuecomment-1233445670

IanPhilips avatar Aug 31 '22 21:08 IanPhilips

Same issue

jomiplaz avatar Sep 06 '22 11:09 jomiplaz