1927945 - IDB: Add implementation for getAllRecords()

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, enhancement, P3)

Description

[Jan Varga [:janv]]

https://chromestatus.com/feature/5124331450138624
https://patrickbrosset.com/articles/2024-11-19-even-faster-indexeddb-reads-with-getallrecords/

Comment 1

[Arnaud Bienner]

Attachment: Bug 1927945 - Add IDBGetAllOptions and getAllRecords definitions to WebIDL and add support for IDBGetAllOptions.query in IDBObjectStore.getAll/getAllKeys. r?janv

Comment 2

[Arnaud Bienner]

Attachment: Bug 1927945 - Implement IDBGetAllOptions' count support for IDBObjectStore getAll/getAllKeys

Comment 3

[Arnaud Bienner]

Attachment: Bug 1927945 - Implement IDBGetAllOptions' direction support for IDBObjectStore getAll/getAllKeys

Comment 4

[Arnaud Bienner]

Attachment: Bug 1927945 - Implement IDBGetAllOptions support for IDBIndex getAll/getAllKeys. r?janv

Refactor code a bit to ease reuse between IDBObjectStore and IDBIndex

Comment 5

[Arnaud Bienner]

Attachment: Bug 1927945 - Implement getAllRecords for IDBIndex and IDBObjectStore. r?janv

Comment 6

[Arnaud Bienner]

I'm following up of the conversion we had on https://phabricator.services.mozilla.com/D295592#10281123 so I can add attachments.

I'm summarizing here my findings regarding the performance analysis of this new function.

First, what is a good dataset for benchmark?

I used dfindexeddb (https://github.com/google/dfindexeddb) to analyze all the IndexedDB I had locally, on my regular profile, which probably represents enough of the average user.

For keys, I had (roughly):

• String: 53%
• Numbers: 27%
• Arrays: 20%

On data types stored, almost everything is an object.

Since there is the opportunity to do some lazy loading, I wanted to assess the impact in terms of performance.

I think there might be some usage where keys are not always accessed, for instance when doing pagination, as described here: https://patrickbrosset.com/articles/2024-11-19-even-faster-indexeddb-reads-with-getallrecords/ (in this case only one key is accessed for each getAllRequest request, to create the next range request).

When key are being accessed, it makes sense to only access one on object store, since primary key and key are the same thing.

So I benchmarked acces value only and key + value. And access all fields too, even if I stated above I don't think there is a real usecase for that on IDBObjectStore.

My benchmark focuses on IDBObjectStore, as I think it is the main usecase. Could do the same thing for IDBIndex if someone thinks this is needed.

To perform lazy loading, there are two options:

• Use [Cached] attribute in .webidl file
  • Pros: less code
  • Cons: can't do aliasing (i.e. have IDBObjectStore's primary key and key points to the same object, since they are the same thing)
• Use caching at the C++ level, like we have for IDBCursor currently: https://searchfox.org/firefox-main/source/dom/indexedDB/IDBCursor.cpp#246
  • Pros: consistent with IDBCursor, can do aliasing for key/primary key
  • Cons: more code

I run some experiment using the attached perf_object.html file, with the 3 approaches:

• No caching at all: IDB Read Performance Tests — ob-os NoCache.png
• [Cached] attribute in webidl: IDB Read Performance Tests — ob-os [Cached].png
• Caching at C++ level (using bool bitfields): IDB Read Performance Tests — ob-os BoolCached.png

Bottom line:

• Caching adds a bit of overhead in the cases where we access key(s) + value
• The two lazy loading approaches gives similar results:
  • [Cached] seems to be slightly more efficient, probably because it avoids hitting the C++ object.
  • BoolCached (IDBCursor approach) is more efficient for the case when all fields are accessed and the key is big. This is expected thanks to the primary/key aliasing, since the key's deserialization happens only once

Note: apart from performance, having lazy loading also allows web apps to be more responsive, if records aren't accessed right away.

Comment 7

[Arnaud Bienner]

Attachment: IDB Read Performance Tests — ob-os [Cached].png

Comment 8

[Arnaud Bienner]

Attachment: IDB Read Performance Tests — ob-os BoolCached.png

Comment 9

[Arnaud Bienner]

Attachment: IDB Read Performance Tests — ob-os NoCache.png

Comment 10

[Arnaud Bienner]

Attachment: perf_object.html

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7fad6f6f6476
https://hg.mozilla.org/integration/autoland/rev/825f0771314a
Add IDBGetAllOptions and getAllRecords definitions to WebIDL and add support for IDBGetAllOptions.query in IDBObjectStore.getAll/getAllKeys. r=webidl,dom-storage-reviewers,smaug,janv
https://github.com/mozilla-firefox/firefox/commit/aa2a80aa3cc5
https://hg.mozilla.org/integration/autoland/rev/76a7c3ec5732
Implement IDBGetAllOptions' direction support for IDBObjectStore getAll/getAllKeys r=dom-storage-reviewers,janv
https://github.com/mozilla-firefox/firefox/commit/90d248b50ef1
https://hg.mozilla.org/integration/autoland/rev/621cd3bee7f0
Implement IDBGetAllOptions support for IDBIndex getAll/getAllKeys. r=janv,dom-storage-reviewers
https://github.com/mozilla-firefox/firefox/commit/946c8bf896ef
https://hg.mozilla.org/integration/autoland/rev/c256727f029f
Implement getAllRecords for IDBIndex and IDBObjectStore. r=janv,webidl,smaug,jari,dom-worker-reviewers,dom-storage-reviewers,asuth,edgar

Comment 13

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/825f0771314a
https://hg.mozilla.org/mozilla-central/rev/76a7c3ec5732
https://hg.mozilla.org/mozilla-central/rev/621cd3bee7f0
https://hg.mozilla.org/mozilla-central/rev/c256727f029f

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/60243 for changes under testing/web-platform/tests

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot