perf(replicache): Batch concurrent get/has operations in SQLiteStore#5955
Open
arv wants to merge 1 commit into
Open
perf(replicache): Batch concurrent get/has operations in SQLiteStore#5955arv wants to merge 1 commit into
arv wants to merge 1 commit into
Conversation
## Summary Optimize `SQLiteStoreRead` by batching concurrent `get()` and `has()` calls into single database queries using microtask scheduling, reducing round-trips when multiple keys are accessed concurrently. ## Key Changes - **Batching in `SQLiteStoreRead`**: `get()` and `has()` calls queue into striped callback arrays (`[resolve, reject, resolve, reject, ...]`) and are flushed together in a `queueMicrotask` callback. `get` and `has` operations are batched independently. - **Single-key fast path**: When exactly one key is pending at flush time, the original single-row SQL (`WHERE key = ?`) is used directly, avoiding JSON serialization overhead for the common sequential case. - **New batch statements**: Added `getMany` (`SELECT key, value FROM entry WHERE key IN (SELECT value FROM json_each(?))`) and `hasMany` (`SELECT key FROM entry WHERE key IN (SELECT value FROM json_each(?))`) prepared statements alongside the existing single-key `get`/`has` statements. - **Updated `PreparedStatement` interface**: Replaced `firstValue()` with `all(params): Promise<unknown[][]>` to support returning multiple rows for batch queries. - **Type aliases for casts**: `GetResolve`, `HasResolve`, `Reject` aliases make the necessary `as`-casts on the `unknown[]` callback array concise. - **All adapters updated**: zero-sqlite, expo-sqlite, and op-sqlite implement the new `all()` method. The expo-sqlite mock's `executeForRawResultAsync` gains `getAllAsync()` support. - **`directory` option**: `SQLiteStoreOptions` gains an optional `directory` field so stores can be created in a specific path; `dropStore`/`dropZeroSQLiteStore` accept it too for consistent file resolution. ## Tests - `sqlite-store.test.node.ts`: Unit tests verifying batching behaviour — concurrent gets coalesce into one `getMany` call, sequential awaited gets use the single-key path, concurrent has coalesces into `hasMany`, mixed get+has use separate SQL calls. - `sqlite-store.test.ts`: Integration test for `SQLiteWrite` batch commit (deletes + upserts flushed as one statement each).
|
@arv is attempting to deploy a commit to the Rocicorp Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves SQLiteStoreRead performance by coalescing concurrent get()/has() calls into batched SQL queries, reducing SQLite round-trips across the Replicache SQLite-backed KV implementations. It also updates the prepared-statement abstraction to support multi-row results and adds an optional directory option for file placement.
Changes:
- Batch concurrent
SQLiteStoreRead.get()and.has()calls viaqueueMicrotask, addinggetMany/hasManyprepared statements plus a single-key fast path. - Update the
PreparedStatementinterface fromfirstValue()toall()and migrate zero-sqlite / expo-sqlite / op-sqlite adapters + mocks. - Add
SQLiteStoreOptions.directoryand thread options through drop helpers/providers; expand test coverage for batching.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/replicache/src/kv/sqlite-store.ts | Implements batched get/has, adds getMany/hasMany, changes PreparedStatement, and introduces directory filename resolution. |
| packages/replicache/src/kv/sqlite-store.test.node.ts | Adds focused unit tests validating batching and single-key fast path behavior. |
| packages/replicache/src/kv/sqlite-store.test.ts | Updates mocks to the new PreparedStatement.all() API and adds batch statement placeholders. |
| packages/replicache/src/kv/sqlite-store-test-util.ts | Adds integration coverage for concurrent get+has correctness across implementations. |
| packages/replicache/src/kv/zero-sqlite/store.ts | Updates drop/provider wiring for options and implements PreparedStatement.all() for zero-sqlite. |
| packages/replicache/src/kv/zero-sqlite/store.test.node.ts | Adjusts node test configuration to use a temp directory via the new directory option. |
| packages/replicache/src/kv/op-sqlite/store.ts | Implements PreparedStatement.all() and updates drop/provider wiring to accept options. |
| packages/replicache/src/kv/expo-sqlite/store.ts | Implements PreparedStatement.all() and updates drop/provider wiring to accept options. |
| packages/replicache/src/kv/expo-sqlite/store.test.node.ts | Updates expo-sqlite mock to support getAllAsync() for all() calls. |
| package-lock.json | Lockfile churn from dependency/install changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
524
to
531
| export function dropStore( | ||
| name: string, | ||
| createDelegate: ( | ||
| filename: string, | ||
| opts?: SQLiteStoreOptions, | ||
| ) => SQLiteDatabase, | ||
| opts?: SQLiteStoreOptions, | ||
| ): Promise<void> { |
Contributor
Author
There was a problem hiding this comment.
@copilot apply changes based on this feedback
Comment on lines
+339
to
+359
| #scheduleLookup(): void { | ||
| if (!this.#scheduled) { | ||
| this.#scheduled = true; | ||
| queueMicrotask(() => { | ||
| this.#scheduled = false; | ||
| const {get, has, getMany, hasMany} = this.#preparedStatements; | ||
| const getKeys = this.#pendingGetKeys.splice(0); | ||
| const getCallbacks = this.#pendingGetCallbacks.splice(0); | ||
| const hasKeys = this.#pendingHasKeys.splice(0); | ||
| const hasCallbacks = this.#pendingHasCallbacks.splice(0); | ||
| if (getKeys.length === 1) { | ||
| void flushSingle(getKeys[0], getCallbacks, get, settleSingleGet); | ||
| } else if (getKeys.length > 1) { | ||
| void flushBatch(getKeys, getCallbacks, getMany, settleGets); | ||
| } | ||
| if (hasKeys.length === 1) { | ||
| void flushSingle(hasKeys[0], hasCallbacks, has, settleSingleHas); | ||
| } else if (hasKeys.length > 1) { | ||
| void flushBatch(hasKeys, hasCallbacks, hasMany, settleHas); | ||
| } | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
@copilot apply changes based on this feedback
Comment on lines
+131
to
+135
| function resolveFilename(name: string, opts?: SQLiteStoreOptions): string { | ||
| const safe = safeFilename(name); | ||
| const dir = opts?.directory; | ||
| return dir ? `${dir}/${safe}` : safe; | ||
| } |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Optimize
SQLiteStoreReadby batching concurrentget()andhas()calls into single database queries using microtask scheduling, reducing round-trips when multiple keys are accessed concurrently.Key Changes
Batching in
SQLiteStoreRead:get()andhas()calls queue into striped callback arrays ([resolve, reject, resolve, reject, ...]) and are flushed together in aqueueMicrotaskcallback.getandhasoperations are batched independently.Single-key fast path: When exactly one key is pending at flush time, the original single-row SQL (
WHERE key = ?) is used directly, avoiding JSON serialization overhead for the common sequential case.New batch statements: Added
getMany(SELECT key, value FROM entry WHERE key IN (SELECT value FROM json_each(?))) andhasMany(SELECT key FROM entry WHERE key IN (SELECT value FROM json_each(?))) prepared statements alongside the existing single-keyget/hasstatements.Updated
PreparedStatementinterface: ReplacedfirstValue()withall(params): Promise<unknown[][]>to support returning multiple rows for batch queries.Type aliases for casts:
GetResolve,HasResolve,Rejectaliases make the necessaryas-casts on theunknown[]callback array concise.All adapters updated: zero-sqlite, expo-sqlite, and op-sqlite implement the new
all()method. The expo-sqlite mock'sexecuteForRawResultAsyncgainsgetAllAsync()support.directoryoption:SQLiteStoreOptionsgains an optionaldirectoryfield so stores can be created in a specific path;dropStore/dropZeroSQLiteStoreaccept it too for consistent file resolution.Tests
sqlite-store.test.node.ts: Unit tests verifying batching behaviour — concurrent gets coalesce into onegetManycall, sequential awaited gets use the single-key path, concurrent has coalesces intohasMany, mixed get+has use separate SQL calls.sqlite-store.test.ts: Integration test forSQLiteWritebatch commit (deletes + upserts flushed as one statement each).