Skip to content
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

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

Open
gmaclennan opened this issue Jul 9, 2016 · 7 comments
Open

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

gmaclennan opened this issue Jul 9, 2016 · 7 comments
Labels
core An issue with one of the core iD components iD v3 Ideas and issues for the next major iD version
Milestone

Comments

@gmaclennan
Copy link
Contributor

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
Copy link
Contributor Author

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

@bhousel
Copy link
Member

bhousel commented Jul 10, 2016

@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
Copy link
Contributor Author

@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
Copy link
Member

bhousel commented Jul 11, 2016

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
Copy link
Member

bhousel commented Jul 13, 2016

@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..

@bhousel bhousel added core An issue with one of the core iD components and removed enhancement labels Nov 1, 2016
@1ec5
Copy link
Collaborator

1ec5 commented Sep 17, 2017

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 bhousel added the good first issue Best for first-time contributors. No experience necessary! label Sep 18, 2017
@bhousel bhousel added help wanted For intermediate contributors, requires investigation or knowledge of iD code wip Work in progress and removed good first issue Best for first-time contributors. No experience necessary! labels Oct 12, 2017
@bhousel
Copy link
Member

bhousel commented Oct 31, 2017

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.

@bhousel bhousel removed Hacktoberfest help wanted For intermediate contributors, requires investigation or knowledge of iD code labels Oct 31, 2017
@bhousel bhousel added iD v3 Ideas and issues for the next major iD version and removed wip Work in progress labels Mar 3, 2018
@quincylvania quincylvania added this to the 3.0.0 milestone Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core An issue with one of the core iD components iD v3 Ideas and issues for the next major iD version
Projects
None yet
Development

No branches or pull requests

4 participants