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

Feature Request: Allow batch adding records (from an Array) #69

Open
klyonrad opened this issue Feb 24, 2016 · 8 comments · May be fixed by #343
Open

Feature Request: Allow batch adding records (from an Array) #69

klyonrad opened this issue Feb 24, 2016 · 8 comments · May be fixed by #343
Labels
feature request needs-pr performance TPAC 2019 Discuss at Fukuoka TPAC2024 Topic for discussion at TPAC 2024
Milestone

Comments

@klyonrad
Copy link

klyonrad commented Feb 24, 2016

I'm not sure if this follows the correct procedure for feature requests, but here I go.

I recently had the issue that I wanted to fill an indexedDB (initially) with a very large Array of Objects. However looping through the Array and adding the Objects one by one was very inefficient. Wouldn't it make sense to have an equivalent of getAll() with adding Records?

@inexorabletash
Copy link
Member

Yeah, a putAll(sequence<value> values, optional sequence<key> keys) is plausible.

We'd have to performance test against the loop. I'm not entirely sure it'd be notably more efficient than the loop in most implementations; if the overhead is in the object serialization and (if necessary) index key generation it doesn't necessarily buy you anything.

For kicks, can you share a minimal example that you'd expect to be significantly more efficient? Then implementers could give it a try and see. (I could whip something up but I might make some assumptions about what you're doing.)

@jfroelich
Copy link

jfroelich commented Nov 2, 2016

In addition to putAll, please also consider batch add/delete. Off the top of my head, something like a couple of these:

  • store.addAll(sequence<value> values) - like store.putAll but error if keys exist
  • store.deleteAll(optional range) - like store.clear() but limited to range
  • store.clear(optional range) - if no range then behave like now, if range then limit deletes to range
  • index.deleteAll(optional range) - limited to store objects referenced by the the index that are within range.

The missing functionality is basically DELETE FROM store WHERE .... As a trivial example, I am working on a simple rss app with stores feeds and articles. When unsubscribing, I open a rw tx over both stores, then delete feed from feeds and articles from articles where article.feedId = feedId. To delete the articles, I must first open a cursor over an index on article.feedId with IDBKeyRange.only(feedId), or use getAll or getAllKeys, then I must iterate over the articles and issue individual deletes. It would be nicer to skip the getAllKeys step, and use a single batch delete request that accepts a range.

@inexorabletash
Copy link
Member

addAll() makes sense along with putAll()

@jfroelich
Copy link

Ah, was basing my thoughts on https://developer.mozilla.org/en-US/docs/Web/API/IDBObjectStore/delete, where it is not clear that store.delete() can be a range and not just a value. Makes perfect sense now. So yes, the use case is specific to 68. Thanks!

@inexorabletash
Copy link
Member

Thanks for the pointer - I've updated the MDN page for both delete and clear to clarify!

@inexorabletash inexorabletash added the TPAC 2019 Discuss at Fukuoka label Jun 5, 2019
@inexorabletash
Copy link
Member

TPAC 2019 Web Apps breakout:

  • Use case makes sense
  • For the key+value case, various options:
  • mumble([v1, v2, ...], [k1, k2, ...]) - terrible ergonomics, rejected
  • mumble({k1:v1, k2:v2, ...}) - this doesn't work because IDB has non-string key types
  • mumble(new Map( .... )) - plausible, maybe the favorite?
  • mumble([[k1, v1], [k2, v2], ...]) - plausible; this is what Map ctor takes
  • mumble([{key:k1, value:v1},{key:k2,value:v2}, ...]) - too wordy?
  • Need to solve put(value) vs. put(value, key) case for multiples; suggestion was to use multiple methods, e.g. putAll() and putAllValues(), such as:
    • putAllValues(iterable)
    • putAll(map) or putAllEntries(map)

Follow-up: concrete proposal, solicit developer feedback

@nolanlawson
Copy link
Member

Is putAll or addAll still in development in Chrome? It's marked as "in development" on the Chrome status page, but based on this comment, it didn't actually improve performance in some benchmark, so it may be abandoned?

@inexorabletash
Copy link
Member

Yeah, the implementation we ended up with didn't show any performance improvements, and we plan to back out the code.

It's possible we didn't spend enough time optimizing the solution. We'd welcome hearing from other implementers if they do see a performance improvement that can't be achieved through other quality-of-implementation work.

@SteveBeckerMSFT SteveBeckerMSFT added the TPAC2024 Topic for discussion at TPAC 2024 label Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request needs-pr performance TPAC 2019 Discuss at Fukuoka TPAC2024 Topic for discussion at TPAC 2024
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants