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

Huge Memory Leak Issue #4416

Closed
Sa-meer opened this issue Feb 4, 2021 · 22 comments
Closed

Huge Memory Leak Issue #4416

Sa-meer opened this issue Feb 4, 2021 · 22 comments

Comments

@Sa-meer
Copy link

Sa-meer commented Feb 4, 2021

[REQUIRED] Describe your environment

  • Firebase SDK version: "firebase": "^8.2.5",
  • Firebase Product: firestore

[REQUIRED] Describe the problem

When I start the onSnapshot() listener on multiple documents the memory utilisation go sky rocket.
So the Browser Tab crashes if there are multiple (10-15) listeners are running simultaneously.

This is how I'm adding my config for firestore:

import firebase from 'firebase/app';
import 'firebase/auth';
import 'firebase/firestore';
  const firebaseApp = firebase.initializeApp({
    apiKey: "",
    authDomain: ,
    databaseURL: "****",
    projectId: "*****",
    storageBucket: "*****",
    messagingSenderId: "******",
    appId: "******"
    })
export const db = firebase.firestore();

There is no processing is going on the data recived from the onSnapshot(), only making the connection and I can see heavy memory usage.
MemoryLeak

On each auto Garbage collection Memory usage drop and as data flows on the listener again it go up and this continues.
GC

@hsubox76
Copy link
Contributor

hsubox76 commented Feb 4, 2021

I'm sure the Firestore team will look into this but since the problem is related to onSnapshot() it will probably be most helpful to see code showing how you are using onSnapshot().

@var-const var-const self-assigned this Feb 4, 2021
@var-const
Copy link
Contributor

Sorry to hear about your troubles. As suggested above, could you please post the code where you're using onSnapshot?

@Sa-meer
Copy link
Author

Sa-meer commented Feb 5, 2021

@hsubox76 @var-const Here is the code sample:
So my data structure on cloud firestore is like this I have a collection and multiple documents which updates quite frequently and I wanna catch those updates and store in my local state of React component.

I'm also storing the returned function from onSnapshot() for unsubscribing in future in an array named unSub so that when I unmount I close all the listeners properly, no leak from there.

documentNameArray.forEach(name => {
     unSub.push(db.collection(collection).doc(name).onSnapshot(this.handleFirebaseUpdates));
 });

handleFirebaseUpdates = (snap) => {
        const snapData = snap.data();
        if (!snapData) return;
        this.setState({ snapData })
 }

I earlier thought that may be it is due to the fact that I'm processing the updates in the handleFirebaseUpdates function so I moved everything away and just storing the data, no operation is performed on frequently coming updates.

But I observe the continuous memory increase on each call of the handleFirebaseUpdates as seen the graph I already attached.

PS: the memory usage increases more and more with number of documents I listen to simultaneously, I expect memory consumption to be a little high around 200-300 Mb but as you see going as high as 929 MB the tab crashes.

@hsubox76
Copy link
Contributor

hsubox76 commented Feb 5, 2021

Can I ask where in your code the documentNameArray.forEach is called? I'm just wondering if there's any chance that loop is being called more times than you expect (sometimes happens in React lifecycle/render methods) and you're opening more listeners than you were expecting to. Can you put a console.log or something above your unSub.push line to double check how many times you're calling onSnapshot when the memory spikes - or if you've already checked that, let us know.

@var-const
Copy link
Contributor

In addition to @hsubox76's question, can you please check whether memory usage still spikes if you comment out this.setState({ snapData }) (or any other similar custom processing that your app is doing)? I have just tried adding ~3K listeners using code similar to the snippet you listed (except for any custom processing), and I saw memory usage increase around 18Mb, so it would be helpful to narrow down the cause as much as possible.

@Sa-meer
Copy link
Author

Sa-meer commented Feb 6, 2021

@hsubox76 Yes I am sure that the loop is only running once and all the listeners are created only once.

@var-const Yes I tried removing everything from the function and still got the memory issue.

One point that i want to add here is that could issue be due to the fact that all the listeners are receiving some big chunk of data at realtime, let's say 200-300 KB on almost all the listeners almost at the same time ??

@var-const
Copy link
Contributor

The size of the docs could contribute to the issue. Can you please clarify:

  • how many listeners do you have? Is the number constant or growing?
  • how often do the listeners receive updates (e.g. once every 2-3 seconds, several times a second, etc.)?
  • what is the average size of a doc (I presume it's 200-300KB, please confirm)?
  • how long does it take after attaching the listeners for the memory usage to go to 900MB+?

@Sa-meer
Copy link
Author

Sa-meer commented Feb 9, 2021

Hi @var-const please follow the details:

  1. Number of listener that I want to work around is 120-150, the number is a variable but for my need max it will go around 200, The above 900+ MB chart is for the 40 Listeners.
  2. Listeners receive updates quite frequently and that is also a little bit variable some times it is multiple times a seconds and some times it is per seconds as well but majorly it is multiple times per second.
  3. Yes the average size of the DOC is approx 300KB.
  4. As the more and more listener gets connected and the data flow on the listener increases (2-3 updates per seconds) the memory starts to go up, I also have seen the memory goes around 1200-1400 MB as well when there are a lot of updates.

@Sa-meer
Copy link
Author

Sa-meer commented Feb 11, 2021

@var-const a gentle reminder over the issue, do you have anything over this ?

@var-const
Copy link
Contributor

var-const commented Feb 11, 2021

@Sa-meer Apologies for the slow reply. I will try to reproduce the issue based on the information you provided today and will post back here. Just to clarify -- are all of the listeners document listeners (attached to a DocumentReference), or are some of them query or collection listeners (attached to a Query or a CollectionReference)?

@Sa-meer
Copy link
Author

Sa-meer commented Feb 12, 2021

@var-const all are the document listeners. no query or collection listeners.

@var-const
Copy link
Contributor

Update: I see memory usage go up to 900+MB in spikes when I use 40 listeners, 300kb documents, and 3 updates per second for each document. I will next check if there are any leaks or any low-hanging fruit to reduce memory usage.

@var-const
Copy link
Contributor

I'm sorry I'm slow with updates. I'm still working on this issue. So far, I don't see any leaks related to listeners or documents -- all memory allocated appears to be garbage-collected. However, I noticed a potential issue with WebChannel -- I need to do more research before I can say whether it's an actual bug or a red herring. I'll keep posting updates as I have them.

@Sa-meer
Copy link
Author

Sa-meer commented Feb 19, 2021

Thanks, please keep me updated because this issue is directly affecting my application performance when data is more frequent and big.

@Sa-meer
Copy link
Author

Sa-meer commented Feb 23, 2021

Hi @var-const any update on this ?

@var-const
Copy link
Contributor

I can confirm that with high traffic, the Web Channel memory consumption can increase significantly (up to 300MB on newer versions of Chrome and even higher than that on older ones). This is because new server responses get attached to the same XmlHttpRequest which only gets recreated periodically (I believe it's every minute). Unfortunately, this isn't something we can fix in the SDK.

I couldn't identify any leaks related to documents or listeners. The memory gets deallocated as expected (except that the latest version of each document gets cached). 300KB docs * 3 updates per second * 40 listeners would result in roughly 35MB of memory usage per second; this adds up quickly, and I presume garbage collection simply doesn't get triggered fast enough to prevent memory usage from increasing significantly.

Perhaps there's a way to restructure your app to reduce the amount of traffic (if you could share some details on your use case)?

@Sa-meer
Copy link
Author

Sa-meer commented Feb 24, 2021

Well the restructuring of the app is always an option but I was looking for a solution with the my current structure, because I'm not doing something very out of the box here just the size and frequency of updates is on a little higher side.

So the common concussion here looks like:
With some frequent big size updates on snapshot listeners (around 40-50 listeners) we will face the high memory consumption and we can not do anything over that, other than reducing the size of updates and frequency I guess because I tested with less amount of data and frequency and for that memory consumption is in an acceptable range.

@var-const
Copy link
Contributor

One thing you can try is to enable long polling in the settings. It's primarily intended to work around issues with proxies and antivirus software, but because it prevents buffering of the responses, it might help here in case a significant part of the memory usage you're seeing comes from the WebChannel buffering. Can you please try that out and let us know if it makes any difference?

@Sa-meer
Copy link
Author

Sa-meer commented Feb 25, 2021

Thanks for this suggestion, I'll try experimentalForceLongPolling and test the same scenario to see if there is any improvement in memory issue.
If it solves the memory issue with some minor performance degradation it will be great but in the documentation itself it says: Use of this option will cause some performance degradation though.

@Sa-meer
Copy link
Author

Sa-meer commented Feb 27, 2021

Hey @var-const so I applied the setting change of experimentalForceLongPolling and it did improved the memory utilisation BUT as mentioned in DOC the performance is gone quite poor, frequent updates are taking more time to reflect and I observed random delay in the updates received on Documents via listener maximum up to 8-10 Seconds !!!

Is there anything else possible to overcome this issue that you can suggest ?

@var-const
Copy link
Contributor

Thank you for reporting your experience -- this does confirm that the memory consumption is significantly affected by the WebChannel issue. Unfortunately, it's an issue in one of our dependencies and not something we can fix.

Forcing long polling makes WebChannel establish a new connection for each document update, so it is expected that updates would take longer; it will also increase the traffic. This brings up another point -- while the current levels of traffic cause problems with memory consumption, they are also likely to be undesirable for users. If there was a way to restructure the data model or the app to reduce the traffic, it will be an improvement in the user experience. One thing that comes to mind -- in a given update, how much of the document is typically modified? If documents have a relatively small part that gets updated frequently and a large part that is only updated occasionally or not at all, splitting such documents into two different documents can help reduce the traffic. I don't know if this applies to your use case, though.

We are always looking for ways to improve the performance, but unfortunately there's nothing in the short term that is likely to make a difference.

@var-const
Copy link
Contributor

Closing the issue for now. If we get an update on the WebChannel issue, I'll post it here.

@firebase firebase locked and limited conversation to collaborators Apr 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants