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

(#5572) - add Safari 10 support for IDB (WIP) #5701

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@nolanlawson
Copy link
Member

nolanlawson commented Sep 17, 2016

This PR is just to start the conversation about supporting Safari 10 IndexedDB. As described in #5572 there are still some issues to resolve:

  • Benchmark to determine if Safari IDB is too slow compared to Safari WebSQL
  • Confirm that the Dexie-reported issues don't affect us
  • Confirm that the UA parsing logic I've added works across all Safari-like browsers, e.g. UIWebView, WKWebView, Chrome iOS, Firefox iOS
  • Add a SauceLabs test for Safari >=10, ensure it passes 100%

It's been a long time coming, but I think it's about time we start putting WebSQL out to pasture.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Sep 17, 2016

For anyone who wants to test this branch:

brew install couchdb # if you haven't
git clone --single-branch --branch safari-10-support \
  --depth 1 \
  https://github.com/pouchdb/pouchdb.git
cd pouchdb
npm install
npm run dev
# open localhost:8000/tests/integration in Safari

@nolanlawson nolanlawson force-pushed the safari-10-support branch from c1d7bc8 to 7f05141 Sep 17, 2016

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Sep 17, 2016

Here's a build of pouchdb.js for folks who would like to drop it into their app to test. (Note that we automatically fall back to WebSQL if we detect that the user has already stored data in WebSQL, so you may need to clear out all browser data.)

@nolanlawson nolanlawson force-pushed the safari-10-support branch from 7f05141 to c119770 Dec 12, 2016

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Dec 12, 2016

I'm having second thoughts about this PR. Tried out some perf tests on my 2013 MacBook Air and seeing huge perf deltas between Safari WebSQL and Safari IDB:

WebSQL IDB Slowdown
basic-inserts 3773 21743 5.76x
temp-views 3383 345380 102.09x

I would run more tests, but I just don't have time to wait for them to finish. The same test that passes in seconds with WebSQL, I wait for 10s of minutes with IDB and it never finishes.

Maybe we should just wait for Safari 11 before we make IDB the default. If we ship a 100x perf regression then users are going to be very unhappy. :(

This was Safari 10.0.2 (12602.3.12) BTW.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Dec 12, 2016

Safari TP (Release 19 (Safari 10.1, WebKit 12603.1.14.2)) is quite a bit faster:

WebSQL IDB Slowdown
basic-inserts 3773 5862 1.55x
temp-views 3383 14650 4.33x

This is more similar to the IDB slowdown we see with Chrome, so seems more acceptable.

@NekR

This comment has been minimized.

Copy link

NekR commented Dec 12, 2016

@nolanlawson what about workers? Does Safari has WebSQL in workers now? If not, maybe makes sense to enable IDB for it only workers.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Dec 12, 2016

No, it does not have WebSQL in workers ( http://html5workertest.com ). My recommendation for that situation is to use pseudo-worker for Safari.

@NekR

This comment has been minimized.

Copy link

NekR commented Dec 12, 2016

@nolanlawson yeah, just thought if things could benefit from using real worker and IDB there. Maybe indeed makes sense to wait till Safari 11.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Dec 12, 2016

The ideal perf scenario per-browser IMO is:

  • Chrome/Firefox: IDB+worker (because of UI thread blocking)
  • Edge/IE: IDB only (it doesn't block UI thread much, no big need for worker)
  • Safari: WebSQL, no worker

In practice just using PouchDB + pseudo-worker for Safari should be fine. This is what I do for https://pokedex.org BTW

@NekR

This comment has been minimized.

Copy link

NekR commented Dec 12, 2016

I see, makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment