Skip to content

perf: build object rows from a cached v8::DictionaryTemplate - #54

Merged
arv merged 2 commits into
mainfrom
arv/dictionary-template-rows
Oct 19, 2026
Merged

arv merged 2 commits into
mainfrom
arv/dictionary-template-rows

Conversation

@arv

@arv arv commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #53.

v8::Object::New() with a list of names makes dictionary-mode objects, which are slow to create and slow to read. Each statement now caches a v8::DictionaryTemplate for its column names. Instances of the template have fast properties and share one hidden class.

  • Lazy creation. The template is built once the statement has produced 16 object rows. Building it costs about as much as 10–20 rows, and building it eagerly made one-shot statements 2–4µs slower.

  • Fallback. These rows keep using v8::Object::New():

    • non-ASCII names: the template reads names as Latin-1;
    • duplicate names and array indices: the template crashes on both;
    • 128 or more columns: V8 gives such instances a dictionary map, which Map::TransitionToDataProperty DCHECKs against, and they would be dictionary-mode objects anyway.

    For every edge case tested, rows have the same own properties, order and values as before.

  • Invalidation. The template is dropped whenever the cached column names are refreshed: on re-prepare, or when the column count changes.

Benchmark

Same setup as #53: Node 22.22.0, 2000-row tables. Runs alternate between builds; each figure is the median of 25 rounds, in µs.

case base #53 this PR vs base
4 cols, all() 1056 1034 768 −27%
4 cols, iterate() 2266 1834 1668 −26%
4 cols, get() ×2000 10223 9702 9850 −4%
10 cols, all() 2087 1976 1477 −29%
10 cols, iterate() 3826 2837 2451 −36%
10 cols, get() ×2000 12252 11142 10676 −13%
one-shot prepare().get() 10.08 10.48 10.32 within noise

In this run, 4-column get() came out slightly slower than #53. The gain from fast properties mostly shows when rows are read, and that benchmark reads only one property per row.

Tests

  • Added a test with unusual column names, each run 100 times so that rows also come from the template. Names covered:
    • duplicates;
    • "0"/"1", "4294967295", "-1", "01", "1.5";
    • __proto__, constructor, length;
    • non-ASCII, empty;
    • quotes, backslashes, newlines.
  • Added a test with 127, 128 and 300 columns.
  • The schema-change tests in perf: cache column names per statement for object rows #53 now run enough rows to hit the template.
  • Node test suite passes: 351 passing, 1 pending.
  • To check that the tests catch real bugs, I removed the template reset on re-prepare: 3 tests failed.

Bun

Bun can't take this path either way. Bun 1.3.5 and 1.4.2 refuse to load the better_sqlite3 addon. The test-bun CI job runs mocha through its Node shebang, so it never actually tests Bun.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WCP5vbpvciRmgWBx2b1zKb

Copilot AI lite review requested due to automatic review settings October 19, 2026 12:06
@arv
arv force-pushed the arv/cache-column-keys branch from 61c1840 to 3fa671b Compare October 19, 2026 12:15
@arv
arv force-pushed the arv/dictionary-template-rows branch from cb404a9 to 7e97ff5 Compare October 19, 2026 12:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Template creation must avoid the 128–252 column range, which can trigger V8 assertion failures or unsupported behavior.

1 open issue
What changed in this PR

This PR improves object-row creation by lazily caching V8 dictionary templates while preserving fallbacks for unsupported column names.

Changes:

  • Adds template creation and invalidation.
  • Builds rows from cached templates.
  • Expands schema-change and unusual-name tests.
File Summary
test/​23.statement.iterate.js Exercises template-backed iteration.
test/​22.statement.all.js Tests template-backed schema changes.
test/​21.statement.get.js Tests template reuse and unusual names.
src/​util/​row-builder.cpp Builds rows from cached templates.
src/​util/​column-keys.cpp Caches templates and handles fallback eligibility.
src/​objects/​statement.hpp Updates the row-builder interface.
src/​objects/​statement.cpp Passes context into row builders.

Review effort: Lite

Comment thread src/util/column-keys.cpp Resolved
Base automatically changed from arv/cache-column-keys to main October 19, 2026 12:23
@arv
arv requested a review from tantaman October 19, 2026 12:24
@arv
arv force-pushed the arv/dictionary-template-rows branch 2 times, most recently from 04457d6 to 57de0a8 Compare October 19, 2026 12:28

arv commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Verify Signed Commit Authors fails on 57164e2 and 57de0a8. Both were signed with this session's SSH key (rebased onto main after #53's squash merge, then the 128-column fix), and that key isn't in allowed_signers. The code is fine; both commits just need to be re-signed with an allowed key, which I can't do from here.


Generated by Claude Code

arv added 2 commits October 19, 2026 14:29
v8::Object::New() with a list of names makes dictionary-mode objects,
which are slower to create and to read. A statement now caches a
v8::DictionaryTemplate for its column names, whose instances have fast
properties and share a hidden class.

The template is created once the statement has built 16 object rows,
since creating it costs about as much as 10-20 rows; statements that
return only a few rows keep the old cost. Names the template can't take
fall back to v8::Object::New(): non-ASCII names (it reads names as
Latin-1), duplicate names and array indices (it crashes on both).
Instances of a v8::DictionaryTemplate with 128 to 252 names start from
the dictionary map V8 uses for large object literals and then go through
Map::TransitionToDataProperty, which DCHECKs that the map is not a
dictionary map. Release builds produce correct rows, but they are
dictionary-mode objects anyway, so the template gains nothing there.
Build such rows with v8::Object::New() as before.
@arv
arv force-pushed the arv/dictionary-template-rows branch from 57de0a8 to 115244b Compare October 19, 2026 12:30
@arv
arv added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 5b70320 Oct 19, 2026
18 checks passed
@arv
arv deleted the arv/dictionary-template-rows branch October 19, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants