perf(zql): cache sql strings too #5273
Open
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Bencher Report| Branch | mlaw/query-cache |
| Testbed | Linux |
Click to view all benchmark results
| Benchmark | File Size | Benchmark Result kilobytes (KB) (Result Δ%) | Upper Boundary kilobytes (KB) (Limit %) |
|---|---|---|---|
| zero-package.tgz | 📈 view plot 🚷 view threshold | 1,771.80 KB(+0.12%)Baseline: 1,769.61 KB | 1,805.00 KB (98.16%) |
| zero.js | 📈 view plot 🚷 view threshold | 239.65 KB(0.00%)Baseline: 239.65 KB | 244.44 KB (98.04%) |
| zero.js.br | 📈 view plot 🚷 view threshold | 65.95 KB(0.00%)Baseline: 65.95 KB | 67.26 KB (98.04%) |
|
| Branch | mlaw/query-cache |
| Testbed | self-hosted |
Click to view all benchmark results
| Benchmark | Throughput | Benchmark Result operations / second (ops/s) x 1e3 (Result Δ%) | Lower Boundary operations / second (ops/s) x 1e3 (Limit %) |
|---|---|---|---|
| src/client/custom.bench.ts > big schema | 📈 view plot 🚷 view threshold | 905.27 ops/s x 1e3(+6.52%)Baseline: 849.83 ops/s x 1e3 | 588.22 ops/s x 1e3 (64.98%) |
| src/client/zero.bench.ts > basics > All 1000 rows x 10 columns (numbers) | 📈 view plot 🚷 view threshold | 2.73 ops/s x 1e3(-0.00%)Baseline: 2.73 ops/s x 1e3 | 2.22 ops/s x 1e3 (81.31%) |
| src/client/zero.bench.ts > pk compare > pk = N | 📈 view plot 🚷 view threshold | 64.51 ops/s x 1e3(+31.44%)Baseline: 49.08 ops/s x 1e3 | 36.81 ops/s x 1e3 (57.07%) |
| src/client/zero.bench.ts > with filter > Lower rows 500 x 10 columns (numbers) | 📈 view plot 🚷 view threshold | 4.04 ops/s x 1e3(+0.93%)Baseline: 4.00 ops/s x 1e3 | 3.47 ops/s x 1e3 (85.82%) |
tantaman
force-pushed
the
mlaw/query-cache
branch
from
f507383 to
18091ac
Compare
tantaman
force-pushed
the
mlaw/query-cache
branch
from
fd51f77 to
33ba982
Compare
tantaman
force-pushed
the
mlaw/query-cache
branch
from
33ba982 to
1d81c87
Compare
tantaman
force-pushed
the
mlaw/query-cache
branch
from
1d81c87 to
c01a6e3
Compare
arv
requested changes
Dec 5, 2025
packages/replicache/package.json
Outdated
| "@op-engineering/op-sqlite": ">=15", | ||
| "@rocicorp/prettier-config": "^0.4.0", | ||
| "@rocicorp/zero-sqlite3": "^1.0.12", | ||
| "@rocicorp/zero-sqlite3": "file:../../../zero-sqlite3/rocicorp-zero-sqlite3-1.0.13.tgz", |
| id: body.id, | ||
| value: result, | ||
| }); | ||
| console.log('WFDFSDFSDFS'); |
| }); | ||
| console.log(result); | ||
| } catch (e) { | ||
| console.log(e); |
tantaman
force-pushed
the
mlaw/query-cache
branch
2 times, most recently
from
ea2a556 to
08c8c15
Compare
tantaman
force-pushed
the
mlaw/query-cache
branch
from
0fee63a to
ff5139b
Compare
arv
approved these changes
Dec 9, 2025
Contributor
arv
left a comment
arv
left a comment
There was a problem hiding this comment.
LGTM
There is a lot of moving parts here. Are you sure that the perf gain is from what you think it is from?
Also, it would be interesting to look at the performance trace to see where we are spending all this the time, We are doing a lot and I think that will impact performance in the end.
| expect(named('num', 123.45).value).toBe(123.45); | ||
| expect(named('bool', true).value).toBe(true); | ||
| expect(named('nil', null).value).toBe(null); | ||
| expect(named('obj', {a: 1}).value).toEqual({a: 1}); |
| /** | ||
| * Formats a SQL query using named placeholders (`:name` syntax). | ||
| * Returns an object with text and a values Record for use with | ||
| * better-sqlite3's named parameter binding. |
Contributor
There was a problem hiding this comment.
How does this impact performance?
Having to create these extra objects everywhere might cause more GC churn?
I guess it should not matter because the positional params also use an object.
Contributor
Author
I'll double check. I'm also hesitant to land this ahead of 0.25 given the gains are modest and the complexity is high. |
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.
We cache prepared statements today but it turns out just constructing the SQL is also expensive.
This now caches SQL strings by
constraint_col_names,start_col_names,basis,dir. So less work than building the full SQL.The big difficulty with this change is figuring out how to bind values into the correct slots. Before we re-recreated the full SQL each time and the library would return values in the correct order for binding.
Example:
But now we have the SQL string pre-cached we need to figure out what slots
constraintandstartvalues go into ourselves.I updated our SQL formatter to support named parameters which makes this easier.
f_{n}fetch. They get named:c_{columnName}fetch. Start slots are named:s_{columnName}Then we can create a binding object:
to pass:
Query: (over ~250,000 issues)
before: 12.62 s/iter
after: 10.32 s/iter
before: planned: track.exists(playlists) 127.35 ms
after: planned: track.exists(playlists) 105.75 ms
Waiting on: rocicorp/zero-sqlite3#19