genkgo / ember-localforage-adapter

Offline usage for Ember Data, based on localstorage adapter, but now uses Mozilla's localforage as data source
Other
133 stars 26 forks source link

Possible memory leak in utils/queue.js #31

Closed iamolivinius closed 9 years ago

iamolivinius commented 9 years ago

Correct me if I'm wrong but I guess there is a memory leak in https://github.com/genkgo/ember-localforage-adapter/blob/master/addon/utils/queue.js

New operations are added to the queue but not removed after they resolve. This leads to an ever-growing queue array which shouldn't be the case for long living web apps.

frederikbosch commented 9 years ago

@iamolivinius That would be correct. This is not an elegant solution. As all records are saved to one localforage key, it must be done sequentially. Therefore I came up with this solution. Best thing to right now, is to solve the memory leak.

Second step might be think of storing the data into multiple keys (one per model). It will increase performance because it can be done async. Then we would also have one queue per model.

frederikbosch commented 9 years ago

Maybe, this is too simple, but it could already help.

    self.queue[queueKey] = new Ember.RSVP.Promise(function(resolve, reject) {
      self.queue[queueKey - 1].then (function () {
        self.queue.splice(queueKey, 1);
        callback(resolve, reject);
      });
    });
frederikbosch commented 9 years ago

Note: there is no specific reason why we store data into one localforage key. This project originated from the localstorage adapter. It was their decision to design the adapter as such.

frederikbosch commented 9 years ago

@iamolivinius Could you try #32? At least Travis thinks it is alright.

iamolivinius commented 9 years ago

Looks good to me. Thanks for the quick response :+1:

frederikbosch commented 9 years ago

Merged. Thanks for the help. Will tag next week.