openstreetmap / iD

🆔 The easy-to-use OpenStreetMap editor in JavaScript.
https://www.openstreetmap.org/edit?editor=id
ISC License
3.38k stars 1.21k forks source link

Switch to async storage (i.e. indexdb instead of localstorage) #3239

Open gmaclennan opened 8 years ago

gmaclennan commented 8 years ago

Currently synchronous localstorage is used, which is fine for most things apart from saving history which can grow very large and causes significant slow-down, despite the being debounced to 350ms.

This is most noticeable when editing/creating complex line/polygon features, which result in the history being large, and writes to localstorage taking significant time, locking up the browser every 350ms. iD quickly becomes unusable in this scenario. This can be solved by frequent committing, but it is not an ideal solution.

To avoid a larger re-factor which would necessitate making all storage calls async, perhaps the quickest/easiest solution would be to use async storage for only history.save().

IndexedDB is the obvious choice. Any recommendations for a lightweight wrapper that gets around any IndexedDB bugs or lack of support on browsers currently supported by iD (looking at you, Safari)? We only need a wrapper that implements basic get() and put().

gmaclennan commented 8 years ago

One option: https://github.com/localForage/localForage Thoughts welcome.

bhousel commented 8 years ago

@gmaclennan Thanks! localForage looks awesome. We can probably use it as an almost drop-in replacement, and looks like it's supported on all modern browsers.

I've noticed that iD's performance gets really bad once we exceed the localStorage limit (see also #2743), so this could be very helpful.

gmaclennan commented 8 years ago

@bhousel where is the list of browsers that iD supports? Couldn't find it but I'm sure I've seen it somewhere. So that we can ensure that any storage option has the same compatibility.

bhousel commented 8 years ago

where is the list of browsers that iD supports?

@gmaclennan We should list this. It's the ones I wrote in #3234

Chrome, Firefox, Safari, IE11, Edge We also have some legacy code to support Opera pre v15 when they switched to the Blink rendering engine. Current versions of Opera are essentially Chromium.

These browsers are all auto-updating now except for IE11, so I don't think too much about specific version numbers.

Safari is currently the crashiest because of #2913

bhousel commented 8 years ago

@gmaclennan If you're interested in working on this, the work would be similar to what I did recently for #3236. Steps are:

  1. Add the library to package.json as a dependency
  2. Where you want to use the library, use an es6 import
  3. You'll want to update this block of code in id.js that manages the localStorage. It probably makes sense to move this code into a module like modules/util/storage.js and add some logic to fallback to localStorage if newer methods return nothing (just so that nobody loses stored history in the switchover).

Are you going to be at SOTM-US this year? I'm looking for ideas for Monday's hackday so this is something I can add to the list and we can work on it together..

1ec5 commented 7 years ago

On more than one occasion, I’ve run into Firefox’s LocalStorage size limit, causing me to lose my changes when the browser crashes or iD gets into a weird state. IndexedDB apparently has a much higher size limit, which would be great for large changes.

In the meantime, if anyone runs into the LocalStorage quota in Firefox, these steps worked for me:

  1. Open about:config and increase dom.storage.default_quota.
  2. Run the following commands in the console on the page containing iD:
    id.history().checkpoint();
    id.history().save();

This sets an entry in LocalStorage with a key based on the domain, e.g. iD_https://www.openstreetmap.org_saved_history.

Typically, at this point, you can close the tab and open iD in a new tab, and your changes will be ready to restore. However, if you don’t want to risk losing your only copy of the changes, you can issue one final command in the malfunctioning instance of iD:

id.history().unlock()

This will make it more likely that the new iD instance will offer to restore your changes.

bhousel commented 7 years ago

See notes on #4418 for where this ended up. Switching to async storage has definite benefits, but would introduce a breaking change in how iD starts.. So this will eventually land whenever we do an iD v3.