swap out idbwrapper with straight indexeddb code

[max-mapper]

idbwrapper has outlived its usefulness to us, and has performance issues

fastest throughput I can get with idbwrapper is 60KB/s. Without it I can get 1MB/s (using substacks idb-blob-store module which uses the IDB API directly)

idb-blob-store chunks messages, so you can probably get even faster with raw idb

[mcollina]

I agree, and not just for speed. That would solve the issue on the null
values as well.
Il giorno dom 6 dic 2015 alle 16:32 James Halliday notifications@github.com
ha scritto:

idb-blob-store chunks messages, so you can probably get even faster with
raw idb

—
Reply to this email directly or view it on GitHub
#46 (comment).

[timkuijsten]

Guys,

I've been working on replacing idb-wrapper with native indexedDB. I do have a working implementation ready[1]. But before I open up a PR I'd like to discuss the approach I've taken which is to store JS types natively and only coerce in _get() or _iterator() if asBuffer is true. Otherwise, if asBuffer is false, always return the native type. Previously this behavior was available via options.raw.

This required only one patch in the tests of abstract-leveldown (which I will submit if you guys think this is the right approach).

Additional changes worth mentioning (see commits in [1]):

• remove unused dependencies and bump the ones that will stay with us

open:

• support multiple stores in the same database
• implement createIfMissing and errorIfExists

destroy:

• delete the given object store, and if no other object stores exist
  in the database, delete the database as well.

batch:

• save keys and values using native JS types, no coercion / encoding
• support sync on batch level, not operation level and only
  supported in Firefox with "dom.indexedDB.experimental" pref set to
  true

iterator:

• If keyAsBuffer or valueAsBuffer, try to coerce most types into a
  String. Drop redundant support for the raw option. Just pass
  keyAsBuffer = false and/or valueAsBuffer = false.
• Make sure that each callback passed to _next() is called exactly
  once, and always in the next tick.
• Support reopening the indexedDB cursor if it timed out before the
  end of the range was reached via reopenOnTimeout option. A snapshot
  is created once per cursor so default to false. Support either snapshot mode (default) or back pressure.

put:

• save keys and values using native JS types, no coercion / encoding
• with experimental support for sync (Firefox only)

get:

• If asBuffer, try to coerce most types into a String. Drop redundant
  support for the raw option. Just pass asBuffer = false.

del:

• with experimental support for sync (Firefox only)

[1] living in https://github.com/timkuijsten/level.js/tree/idbunwrapper

[timkuijsten]

According to observations noted in #51 (comment) (aggressive IndexedDB cursor timeouts) I've reworked the iterator to reopen a cursor if the stream is not ended and the end of the cursor was not reached.

[timkuijsten]

Just realized that automatically reopening a new cursor on a transaction timeout breaks the default snapshot property of leveldb and indexeddb. I've added the reopenOnTimeout option which defaults to false so that the default behavior mimics that of LevelDB (at the cost of getting a TransactionInactiveError if you're calling next not fast enough, which currently breaks some abstract tests).

[vweevers]

at the cost of getting a TransactionInactiveError if you're calling next not fast enough

It would be better to cache cursor results, so that it doesn't matter when next() is called, it can simply take a kv pair from the cache. This will make the behavior similar to leveldown and others, and not break streams (built on iterators) for example. I've got a sample implementation here.

I've added the reopenOnTimeout option

How about naming it snapshot, which defaults to true. If false, it keeps reading until the cache reaches a certain size (a highWaterMark like in streams). One can keep nexting until the cache is empty, after which a new cursor is created, and the cache refilled.

[timkuijsten]

It would be better to cache cursor results, so that it doesn't matter when next() is called, it can simply take a kv pair from the cache. This will make the behavior similar to leveldown and others, and not break streams (built on iterators) for example.

and

How about naming it snapshot, which defaults to true.

I like the idea in general, but I'm not sure what cache.push and shift for every item do to performance/GC. Furthermore, if a large database is iterated with a slow stream consumer then big memory spikes might occur.

... If false, it keeps reading until the cache reaches a certain size (a highWaterMark like in streams). One can keep nexting until the cache is empty, after which a new cursor is created, and the cache refilled.

Sounds good. Would be nice to have some feedback from one of the maintainers.

[vweevers]

if a large database is iterated with a slow stream consumer then big memory spikes might occur.

No way around that, sadly. One can get proper snapshotting or proper backpressure, but not both. All level.js can do is offer a choice.

[max-mapper]

Just wanted to chime in to say apologies I haven't been able to dive in to provide feedback on this yet, I think the work here is really awesome so far.

One question I have re: snapshots on top of IDB in general (not something I've thought about much until this point): Does creating a new cursor mean creating a snapshot based at a later point in time? Or can you create a cursor pointing at some old state to at least get the same read state out of the db as the previous one.

Re: the caching proposal, I do think we should find a way to paper over the TransactionInactiveError issue so that users of e.g. leveldown backed by level.js won't have to know/worry about it. I think in general a default mode should try to prioritize compatibility with modules that will be running in node on leveldown, so we should try to make snapshot semantics match as much as possible. If people want to tune performance in the browser it should be an opt-in.

[vweevers]

Does creating a new cursor mean creating a snapshot based at a later point in time?

Yes. The implementation might not use snapshots, but effectively, for the purposes of this discussion, yes. It means creating a new readonly transaction, which guarantees:

As long as a "readonly" transaction is running, the data that the implementation returns [..] must remain constant. There are a number of ways that an implementation ensures this. The implementation can prevent any "readwrite" transaction [..] until the "readonly" transaction finishes. Or the implementation can allow the "readonly" transaction to see a snapshot of the contents of the object stores which is taken when the "readonly" transaction started.