Performance overhead for non-change actions

[kpdecker]

Under the 0.x code, and as best I can tell 1.x, it looks like object serialization is run for every single action in the system, regardless of if it changes the state or not. This could lead to overhead both in the JSON stringify calls and the blocking Storage calls.

I'm wondering if it's worthwhile for the storage logic to perform pure checks to see if the state has changed to help improve the general performance of the app, particularly when used in conjunction with libraries like redux-form that can create actions on every keypress.

[RanzQ]

Noticed the same issue when added a heartbeat action that triggers every second. +1

[hampsterx]

Just to clarify: state is not deeply cloned on every action. Only the parts that changed are cloned (again, not deeply—depends on what changed). For example, when a todo is edited in TodoMVC app, only that todo object is cloned. The rest of the todo objects are the same. Of course, a root new todo list array is created, pointing to the new object, but the objects themselves are not cloned if they have not changed. - https://github.com/reactjs/redux/issues/758

Looks like just need to compare (key by key) the previous state to the new state to see if it needs to persist or not. heh, that will be compatible even if using an immutable library

might have a poke on weekend, in same situation with timer running every few seconds hah.

[hampsterx]

https://github.com/hampsterx/redux-localstorage/commit/7f0407b8d03023972f1076596738e0e152f29914

Won't do a PR as it's on the master branch, not sure how it should be done on 1.x branch

will suffice for my own needs, author's last commit was 4 months ago :(

[thien-do]

I agree. This is a real issue. @hampsterx 's solution works great for me.

It seems 1.x is stable now. @elgerlambert can you please tell us how should we integrate this into 1.x? I can do the pull request if you busy, but at the moment I don't know how (or where, to be exact)

[elgerlambert]

Hi guys,

The design decisions that have lead to the direction of 1.x were based on solving this particular issue in an unopinionated way. Storage enhancers allow you to "insert" your own logic in between fetching & persisting the data.

To solve this issue with 1.x you would create a storage enhancer that (reference) checks the state passed to storage.put and either returns if nothing has changed or persist if it has.

I haven't had the chance to put it through its paces and implement this in production yet, but my approach for a project I'm working on will be to create a storage enhancer that splits the store state so that each reducer "slice" is persisted separately, followed by a second storage enhancer that reference checks each slice so that it is only persisted if it's changed. (I'll open source these when implemented)

[neeharv]

@elgerlambert any examples of how one can do this?

[elgerlambert]

Haven't found the time to pick up where I left off, but this is the code I was working on (please note that the code below is WIP):

function splitReducers(storage) {
  return {
    ...storage,

    put(prefix, state, callback) {
      let error;
      let count = 1;
      const keys = Object.keys(state);

      const done = err => {
        if (err) { error = err; callback(err); return; }
        count--;
        if (!count && !error) callback();
      };

      storage.put(`${prefix}/_keys`, keys, (err) => {
        if (err) { callback(err); return; }
        keys.forEach(key => {
          count++;
          storage.put(`${prefix}/${key}`, state[key], done);
        });
        done();
      });
    },

    get(prefix, callback) {
      let error;
      let count = 1;
      const persistedState = {};

      const done = (key, err, value) => {
        if (err) { error = err; callback(err); return; }
        count--;
        if (value) persistedState[key] = value;
        if (!count && !error) callback(null, persistedState);
      };

      storage.get(`${prefix}/_keys`, (err, keys) => {
        if (!keys) { callback(err); return; }
        keys.forEach(key => {
          count++;
          storage.get(`${prefix}/${key}`, done.bind(null, key));
        });
        done();
      });
    }
  };
}

In combination with:

function pureMixin(storage) {
  const previousState = {};
  return {
    ...storage,

    put(key, state, callback) {
      if (previousState[key] === state) { callback(); return; }
      console.log('Persisting', key);
      previousState[key] = state;
      storage.put(key, state, callback);
    },

    get(key, callback) {
      storage.get(key, (err, state) => {
        previousState[key] = state;
        callback(err, state);
      });
    }
  };
}

Apply these as you would any storage enhancer:

export default compose(
  splitReducers,
  pureMixin
)(adapter(localforage));

With the current implementation the _keys object is still persisted every time..

[legomushroom]

Hey, guys!

I have a similar issue. I use redux to build animation tools and pretty big apps. Performance gets really bad with a lot of actions and a large store because you have to serialize data and access localStorage on every fired action. Of course, in my case, this can overlap with background animations which start to hang and jabber.

My solution over this was to save/load the state exactly once - when a user leaves a page and when a page is loaded. So I have a unified pageHide that saves the state.

Are you planning/considering something like that? Are you up for a PR?

Thanks a lot!

[elgerlambert]

Hi @legomushroom,

Are you effectively saying that you've "whitelisted" a particular event and that your state is only persisted when this event is dispatched? If you could share some code that could help me better understand what you've done, that would be great!

As a side note I would also suggest looking at something like (Mozilla's) localForage for example. localStorage only provides a synchronous (blocking) api that requires all the data to be serialized before persisting. localForage provides an asynchronous (non-blocking) api that doesn't require store state to be JSON.serialized before hand. So this should definitely help in your case since animations will be particularly sensitive to localStorage's blocking api. (requires v1.0.0-rc)

[legomushroom]

Hi @elgerlambert !

The simplified code is:

window.addEventListener('beforeunload', saveStore);

Of course in cross-browser manner so I target more events. The idea is that you save your store only once - when a user leaves the page.

Thanks a lot for pointing out the localForage, love it. Will try shortly.

Cheers!

[steveliles]

I'm just trying out redux-localstorage on a React-Native project with AsyncStorage and ImmutableJS, so my use-case is a little different to the others in this thread, however I found perf to be miserable until I added debounce.

I figured it was worth a mention here, since it goes a long way towards addressing the problem described here (serialization on every action) but is a little less all-or-nothing than @legomushroom's approach.

Of course it would be much sweeter if slices of state were stored separately and only when modified (per @elgerlambert's wip code above), but debounce already appears to reduce the problem to the point of being unnoticeable to me.

const storage = compose(
  debounce(500),
  serialize
)(adapter(AsyncStorage));