perf: hoist row-invariant column derivation out of bulk INSERT render - #14
Merged
Conversation
Bulk INSERT was the single largest CPU cost in Cygnet's render path — ~855 ns/row, ~15x any other per-row cost — because _render_bulk_insert called _extract_insert_fields once per row, rebuilding a throwaway kwargs dict plus two attribute sets on every row. Derive the emitted column shape once from the first object, then for each subsequent row replay the same field-order classification inline (a precomputed (attr, column, is_dbkey, is_pk) plan + direct getattr). _extract_insert_fields is unchanged (still shared with the single-row INSERT path); only the bulk per-row loop changed, and the first row still goes through it so first-row error semantics are intact. Render bulk-100 87us -> 53us (~1.64x); full path 106us -> 67us. Output is byte-identical — SQL, params, and exception type/message, including the AppKey-None-before-shape-mismatch error ordering — verified by a new characterization net (tests/test_bulk_insert_shape.py, 8 tests), an exhaustive 198-scenario old-vs-new differential, the full 512-test unit suite, and adversarial review. A looser variant measured faster but silently dropped the per-row AppKey-None check; rejected. Also adds bench/profile_hotspots.py (cProfile self-time + timeit ns/op, FakeDB-isolated, drives await via gen.send(None) to stay pure-Cygnet), a `just profile` target, and gitignores its --save .prof output.
Xof
added a commit
that referenced
this pull request
Jun 27, 2026
Follow-up to #14. ARCHITECTURE.md gains a landmine: _render_bulk_insert's per-row loop replicates _extract_insert_fields' field-order classification inline for speed and does not call it per row, so the two must stay byte-identical (same omit/raise rules, AppKey-None short-circuit before a shape mismatch). THEORY.md gains the write-path-hotspot narrative as a sibling to the read-path hydration one, including why the faster looser variant (which dropped the per-row AppKey-None check) was rejected. Both cross-reference ADR PF1; no duplication between the two docs.
Xof
added a commit
that referenced
this pull request
Jun 30, 2026
Re-ran the cross-ORM comparison (Cygnet/SA/Django x psycopg/asyncpg) on the documented setup (M3 Max, Dockerised PG 16, fsync=off, single connection), median of five runs, and refreshed the README table. Added a 1000-row bulk INSERT row. At that batch size Cygnet's single multi-row VALUES statement is ~2x faster than Django bulk_create and ~3x faster than SQLAlchemy's unit-of-work, and a driver crossover appears: psycopg and asyncpg are neck-and-neck on small ops, but at 1000 rows asyncpg is ~1.8x ahead because psycopg's $N->%s placeholder translation is a regex pass over the (now large) SQL string. Harness fix: the bulk-INSERT benchmarks inserted into the shared seeded accounts table with no cleanup, so rows accumulated across rounds and columns -- inflating the bulk numbers and skewing the JOIN benchmark that runs after (it had crept to ~3,800us purely from table bloat). The bulk benchmarks now reset to the seed before each round via benchmark.pedantic, so every batch and every ORM inserts into the same clean state (JOIN back to ~2,100us). TestBulkInsert is size-parametric (N class attr) and TestBulkInsert1000 reuses it for the 1000-row variant. The render-level bulk-INSERT hoist (#14) is a ~1.64x render win but only ~6% of e2e even at 1000 rows, so it does not surface in this round-trip-dominated table; it lives in the render microbenchmark.
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
1 participant
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.
What
Bulk INSERT was the single largest CPU cost in Cygnet's render path.
_render_bulk_insertcalled_extract_insert_fieldsonce per row, rebuilding a throwaway kwargs dict plus two attribute sets on every row — even though the emitted column shape is fixed by the first row (the bulk same-shape contract).This derives the column shape once from the first object, then for each subsequent row replays the same field-order classification inline (a precomputed
(attr, column, is_dbkey, is_pk)plan + directgetattr)._extract_insert_fieldsis unchanged — it's shared with the single-row INSERT path; only the bulk per-row loop changed, and the first row still goes through it so first-row error semantics are intact.Why it was the hotspot
Profiled with the new
bench/profile_hotspots.py(cProfile self-time +timeitns/op, FakeDB-isolated): the per-row_extract_insert_fieldscall was ~855 ns/row, ~15× any other per-row cost in the codebase.Result
per-row marginal cost ~855 → ~500 ns/row. No other operation changed.
Correctness — byte-identical output
This is a behavior-preserving refactor. Verified that SQL, params, and exception type/message/ordering are identical to the old code:
tests/test_bulk_insert_shape.py(8 tests): exact SQL + param ordering, both shape-mismatch directions, AppKey-None in a later row, explicit-PK, single-object bulk._extract_insert_fields) and compared old-vs-new across 198 DBKey/AppKey scenarios + type errors: identical on every one.A looser variant measured ~2.1× but silently dropped the per-row AppKey-None check (emitting
NULLinstead of raising) — a correctness regression. Rejected.Also included
bench/profile_hotspots.py— standalone hotspot profiler complementing the pytest-benchmark suite (it answers "which function owns the time?";just benchanswers "did it get slower?"). Drivesawait buildervia a singlegen.send(None)so the profile stays pure-Cygnet (no asyncio frames).just profiletarget;bench/.profiles/gitignored.Follow-up (not in this PR)
The README cross-ORM table's bulk-INSERT cells (
Cygnet/psycopg 469,Cygnet/asyncpg 394µs) will shift ~4% in Cygnet's favor. They're real-PG full-stack numbers (Docker PG + Django + SA + asyncpg) explicitly framed as "informal, single-run, orders of magnitude" — re-running only Cygnet's cells would make them inconsistent with the un-rerun SA/Django cells, so they should be refreshed on the next full comparison run rather than hand-edited.