-
Notifications
You must be signed in to change notification settings - Fork 107
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
Performance overhead for non-change actions #29
Comments
Noticed the same issue when added a heartbeat action that triggers every second. +1 |
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. |
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 :( |
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) |
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) |
@elgerlambert any examples of how one can do this? |
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):
In combination with:
Apply these as you would any storage enhancer:
With the current implementation the |
Hey, guys! I have a similar issue. I use My solution over this was to Are you planning/considering something like that? Are you up for a Thanks a lot! |
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 |
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 Cheers! |
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.
|
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.
The text was updated successfully, but these errors were encountered: