adriancarriger / angularfire2-offline

🔌 A simple wrapper for AngularFire2 to read and write to Firebase while offline, even after a complete refresh.
https://angularfire2-offline.firebaseapp.com/
MIT License
207 stars 48 forks source link

Huge Memory Usage (AfoListObservable) #51

Closed trenthm closed 7 years ago

trenthm commented 7 years ago

New to Angular, AngularFire2, and AngularFire2 Offline, so please forgive my ignorance 😃

The Issue

When using Reference.update() on a an AfoListObservable with even a modestly sized object the memory usage gets huge.

Using update() with a 1500 key object uses 1+ GB of memory (based on the Chrome Task Manager). Users of my application will frequently upload lists with 1,000s of items.

I found it while experimenting with the best way to store long lists of items. (Even made a stackoverflow question about it.)

Code to Reproduce

this.bigList = this.afoDatabase.list(`big_list/${uid}`);

const listRef = this.fbApp
  .database()
  .ref(`big_list/${uid}`);

for (var i = 0; i < 1500; ++i) {
  updates[listRef.push().key] = `${i}teststring`;
}

listRef
  .update(updates)
  .then(_ => {
    console.log("Finished updates");
  })
  .catch(function(error) {
    console.log("Updates failed: " + error.message);
  });

Workaround

Using afoDatabase.object() instead of afoDatabase.list() seems to work fine. The update() operation happens quickly and the memory barely jumps.


Oh, and big thanks to @adriancarriger for this library. It's looking very promising and sooo easy to use.

P.S. I'm really hoping I'm not missing something super obvious

adriancarriger commented 7 years ago

Hi @trenthm, thanks for reporting this! I'll be looking into it soon! 👍

adriancarriger commented 7 years ago

Code updates

Hi @trenthm, I noticed a few things in your code that might improve things

Changes

Code examples

Performance

I made a change in Afo version 4.2.4 that should reduce the memory usage. There are still potential areas for performance improvements, but with this change I'm able add 5000 items without an issue, while beforehand the operation wouldn't complete.

Demos

Here are some demos you can try to compare performance (it might help to only run one demo at a time)

Next steps

Let me know if this significantly reduces the memory consumption for your use case. Thanks! 👍

trenthm commented 7 years ago

I was under the impression that the following code (from my OP) generated unique keys on the client side, without making multiple server calls (stackoverflow link – see second comment on answer):

for (var i = 0; i < 1500; ++i) {
  updates[listRef.push().key] = `${i}teststring`;
}

Is that incorrect? I notice that all of your demo code uses push(value) (with a value), not push() (without value). https://firebase.google.com/docs/reference/js/firebase.database.Reference#push

For my use case, I've decided to store the list of items as a JSON.stringify'd array, so I can't really test it any better than you have.

If you think it's solved, then this is closed as far as I can tell.


btw, awesome response. I have big hope for this library's future! It's already saved me so many hours. (Why not get a donation link on the projects front page... I'd be happy to throw a few :beer:'s your way)

adriancarriger commented 7 years ago

Hi @trenthm, I didn't realize you were using the base Firebase Javascript SDK.. I was actually thinking you were using AngularFire2 which this library wraps. You're certainly right that you can call .push() without any arguments using the base SDK.

If you need Firebase access for parts of your app that you don't want data stored offline, you can use AngularFire2 alongside AngularFire2 Offline and the methods are mostly the same.

I'm glad this helped resolve your issue and it's great to hear that this library is saving you time! 🎉 👍