Improve performance by reducing number of inputs from / outputs to localStorage

[Leooo]

Hello,

As of now the performance of the LSAdapter seems not great, due to the fact that each record fetched from / saved to the store is a direct copy of the one in localStorage.

Indeed the whole store-hash has to be loaded / saved to the localStorage everytime (and due to the namespace parameter one can not benefit from the localStorage.getItem / setItem methods). And as the localStorage operations are synchronous, this freezes the screen of the users.

One solution for at least some people, and probably any big application using LSAdapter would be to fetch the localStorage hash on initialization, and work on a javascript object copy of this hash, updating it for each request and storing it only at determined intervals. We would admittedly lose some information if the page is reloaded before saving the last copy, but providing the users with a method to force the update of the localStorage when needed would solve this problem.

Below is an implementation of such a method.

Benchmarking on a couple hundred objects + relationships fetched from the server using SSE:

• before implementation (or flag useSnapshot=false): 1.2 seconds for 946 calls to persistData().
• after (flag useSnapshot=true): ~40 ms for 946 calls to persistData().

So the performance seems to be improved by a factor of 30.

App.ApplicationAdapter = DS.LSAdapter.extend({
  namespace:"ember",
  timeoutSnapshot:1000,
  useSnapshot:true
});

//call with $store.adapterFor(App)
//$store.adapterFor(App).pushLocalStorage to force push to localStorage

DS.LSAdapter.reopen(Ember.Evented,{
    storage: null,
    _useSnapshot: function () {
      return (typeof(this.get('useSnapshot'))==="undefined" ? false : this.get('useSnapshot'));
    },
    hasChanged: false,
    _timeoutSnapshot: function () {
      return this.get('timeoutSnapshot') || 10000;
    },
    init: function(){
      this.loopPushLocalStorage();
    },
    loadData: function () {
      var storage,arr;
      if (!this.storage||!this._useSnapshot()) {
        storage = localStorage.getItem(this.adapterNamespace());
        arr = storage ? JSON.parse(storage) : {};
        if (this._useSnapshot()) this.storage=arr;
      }
      else {
        arr=this.storage;
      }
      return Ember.copy(arr,true);
    },
    loopPushLocalStorage: function() {
      this.pushLocalStorage();
      var self = this;
      Em.run.later(function(){
        self.loopPushLocalStorage();
      }, self._timeoutSnapshot());
    },
    pushLocalStorage: function(){
      //timer_begin("persist_data");
      if (!this._useSnapshot() || !this.storage) return false;
      if (this.hasChanged) {
        localStorage.setItem(this.adapterNamespace(), JSON.stringify(this.storage));
        this.hasChanged=false;
      }
      //timer_end("persist_data");
    },

    persistData: function(type, data) {
      //timer_begin("persist_data");
      var modelNamespace = this.modelNamespace(type);
      if (this._useSnapshot()) {
        this.storage=this.storage||this.loadData();
        this.storage[modelNamespace] = data;
        this.hasChanged=true;
      }
      else {
        var localStorageData = this.loadData();
        localStorageData[modelNamespace] = data;
        localStorage.setItem(this.adapterNamespace(), this.JSONStringify(localStorageData));
      }
      //timer_end("persist_data");
    },

    _namespaceForType: function (type) {
        var namespace = this.modelNamespace(type);
        if (!this._storage) {this._storage=this.loadData();}
        var storage   = this._storage;

        return storage[namespace] || {records: {}};
      },
});

EDIT2 Added Ember.copy(x,true) as default implementation of Ember.copy doesn't deep-copy hashes!.

[kurko]

I love it. I think, though, that can't be the default. I'd love to see this in an Ember.js app:

DS.LSAdapter.reopen({
  enableFastPersistence: true
});

and then, the code would be routed inside of the adapter to use the one you have above. Also, it should not be inside of the adapter; it should be its own object to keep things sane and clean.

What do you think?

[Leooo]

Sound reasonable, as long as users of the LSAdapter are aware that using default settings will very probably slow down their ember app a lot.

[kurko]

We can add it to the README. Not losing data is more important than speed.

Em qua, 25 de mar de 2015 às 13:33, Leooo [email protected]
escreveu:

Sound reasonable, as long as users of the LSAdapter are aware that using
default settings will very probably slow down their ember app a lot.

—
Reply to this email directly or view it on GitHub
#105 (comment)
.

[Leooo]

• There are other solutions which would allow to keep all data while improving performance I think:
  1. loading from Local Storage only on page init (and save as a JS object).
  2. then finding records straight from the JS object
  3. when saving/updating, don't reload first from LS, instead use the JS object and update it, and then push it to LocalStorage.
• Then, I'm novice to Ember and its loops process, but probably the right solution is to save to LocalStorage only after other actions have finished: i.e. if like me people want to use LS as a cache and load data from SSE, I only want to save at the end of each batch of records loaded from server (but once again, for this in my case I'm happy to deal with the .pushLocalStorage() function myself). On my understanding it is what is used to refresh templates when loading lots of data: the bindings are refreshed only after all data have been loaded (?)
• For the readme thing, when I started learning ember I was reading threads of ppl who where not happy of the performance compared to Angular, and who were using LSAdapter to test their Ember config. Probably good for such novice users' understanding (like me) to print clearly that LSAdapter slows down Ember compared to fixtures or server loading of records.

[Leooo]

Update on this, as I think this is a huge problem with the localstorage adapter to push /pull to the localstorage for every request (see performance times above).

I now use a _hasChanged property on the adapter to flag when new data are pushed, and a debounced observer to push to localStorage when data are changed. The performance is very good compared to before, and with the debounced observer with a low timeoutSnapshot (I use 200 ms) there is epsilon risk of losing data when reloading the page.

I'm not good with pull requests, but the code itself is quite short, I hope I'm adding everything this is as below:

DS.LSAdapter.reopen({
  _storage: null,//where the copy of the data of localStorage will be stored
  useSnapshot: true,//set to false if ever anyone would like to come back to previous behavior
  timeOutSnapshot: 200,//play with the parameter, shorter time = performance decreased, higher value=risk of losing data when reloading the page
  _hasChanged: false,//flag to detect pushed data,
  loadData: function () {
    var storage,arr,arr_cp;
    if (!this._storage||!this.useSnapshot) {
      storage = this.getLocalStorage().getItem(this.adapterNamespace());
      arr = storage ? JSON.parse(storage) : {};
      if (this.useSnapshot) {
        this._storage=arr;
      }
    }
    else {
      arr=this._storage;
    }
    var cp=Ember.copy(arr,true);
    return cp;
  },
  timerPushLocalStorage: null,
  _launchPushLocalStorage: function() {
    if (this.get('_hasChanged')) {
      Ember.run.debounce(this,'_pushLocalStorage',this.timeoutSnapshot);
    }
  }.observes('_hasChanged'),
  _pushLocalStorage: function() {
    if (!this.useSnapshot || !this._storage) {return false;}
    if (this.get('_hasChanged')) {
      this.getLocalStorage().setItem(this.adapterNamespace(), JSON.stringify(this._storage));
      this.set('_hasChanged',false);
    }
  },
  persistData: function(modelClass, data) {
    var modelNamespace = this.modelNamespace(modelClass);
    if (this.useSnapshot) {
      if (!this._storage) {this._storage=this.loadData();}
      this._storage[modelNamespace] = Ember.copy(data);
      this.set('_hasChanged',true);
    }
    else {
      var localStorageData = this.loadData();
      localStorageData[modelNamespace] = data;
      this.getLocalStorage().setItem(this.adapterNamespace(), JSON.stringify(localStorageData));
    }
  },

[kurko]

@Leooo the code is a very good start. No one starts good at anything. Give a try in the PR and we can guide you through the process. No one best than you to push forward this nice solution.

[Leooo]

Done, but my version of the adapter diverged too much to be able to merge it: I pulled the last LS adapter version and added changes which I think should be self-contained.