perf: cache parsed CREATE TABLE constraints in codegen (#643) - #649
Merged
Conversation
Obligation ids in tests/mcdc/obligations.json are <file>_<line>; the decisions at src/vdbe/exec.rs previously tagged exec_444/exec_664 drifted to exec_463/exec_647 as the file grew. Rename the existing MC/DC vector tests to match so make test-mcdc discharges them again. spend: trivial, matched estimate
…pile_update_with_catalog (#643) compile_insert and compile_update_with_catalog both re-tokenized and re-parsed schema.sql on every call purely to recover CHECK/PRIMARY KEY/AUTOINCREMENT info. Add a process-wide, content-addressed cache (cached_create_table, keyed by schema.sql) so the same DDL text is only parsed once across however many statement compiles reuse it — e.g. exec.rs's multi-statement script mode. TableSchema itself can't carry this cache: src/schema/ddl_reader.rs must have zero dependency on src/parser (spec 002 Requirement 5, verified), so the cache lives in codegen instead, following the existing OnceLock<Mutex<HashMap<...>>> pattern from src/vfs/shm.rs. Note: insert_single/update_pk in tests/performance/crud.rs show no measurable change from this — that benchmark compiles the program once outside its timed loop, and even inline, parse_create_table costs ~5µs against ~14ms/iter of I/O-dominated execution time. The premise that this reparse was a major contributor to the oracle perf gap doesn't hold up under measurement; the cache is still correct and useful for workloads that compile the same schema repeatedly. spend: matched estimate (small) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
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.
Summary
compile_insert/compile_update_with_catalogboth re-tokenized and re-parsedschema.sqlon every call to recover CHECK/PRIMARY KEY/AUTOINCREMENT constraint info. Addscached_create_tableinsrc/codegen/stmt/insert.rs: a process-wide, content-addressed cache (OnceLock<Mutex<HashMap<String, Arc<CreateTable>>>>, same pattern assrc/vfs/shm.rs'sSHM_FILES), keyed byschema.sql, so the same DDL text is parsed once no matter how many statement compiles reuse it.TableSchema(src/schema/ddl_reader.rs). That's not possible —ddl_reader.rsmust have zero dependency onsrc/parser(spec 002 Requirement 5, verified by its own scenario test). The cache instead lives at the codegen layer, where bothTableSchemaand parser types are already used.insert_single/update_pkintests/performance/crud.rsshow no measurable change from this fix. Measured in isolation,parse_create_tablecosts ~5µs; those benchmarks run ~14ms/iteration dominated by fsync/journal/B-tree I/O, andbench_writecompiles the program once outside its timediter_batchedloop anyway — so the reparse this ticket targets was never in that benchmark's hot path, and wouldn't be visible even if it were. The premise that this reparse was "a likely major contributor to a ~10x perf gap" doesn't hold up under measurement. The cache is still correct and useful for workloads that compile the same schema repeatedly (e.g.exec.rs's multi-statement script mode running many single-row INSERT/UPDATEs).Refs: #643
Test plan
cargo test— full suite passes (959 lib tests + all integration suites, 0 failures)cargo clippy --all-targets— cleancargo fmt --check— cleanparse_create_tablecost in isolation (~5µs) vs.insert_single/update_pkbench iteration cost (~14ms) to verify/refute the ticket's perf premise before claiming a benchmark winspend: matched estimate (small)
🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com