perf: batch edge-creation endpoint checks in bulkCreate/bulkInsert#227
Merged
Conversation
executeEdgeCreateNoReturnBatch/executeEdgeCreateBatch validated each edge's from/to endpoints with a getNode() call per edge — for a batch with mostly-unique endpoints (the common bulk-load case), that's thousands of individual round trips per batch instead of one batched fetch per distinct node kind. Node creation already had this batched prefetch via primeBatchValidationCaches; edge creation never got the equivalent treatment. Found profiling a real LDBC SNB SF1 bulk load (packages/benchmarks) that was far slower on TypeGraph/SQLite and TypeGraph/Postgres than on Neo4j or LadybugDB. A controlled 1M-row synthetic reproduction isolated it: hasCreator edge-batch time grew from ~90ms to ~630ms per 2,000-row batch as the graph grew, while an equivalent node-only batch (no edges) stayed roughly flat. Adds seedEndpointRow to createEdgeBatchValidationBackend (mirroring seedNodeRow on the node side) and primeEdgeBatchValidationCache, which collects every distinct (kind, id) pair referenced across a batch's from/to endpoints and issues one getNodes() call per kind before the per-edge validation loop runs. Both batch paths now share a prepareEdgeBatchCreates helper (matching the node side's prepareBatchCreates) instead of duplicating the loop. No behavior change — pure internal optimization. Updates one existing test that asserted the old per-edge getNode() count (2 calls) to assert the new batched getNodes() count instead (also 2, one per kind, but zero individual getNode() calls). Verified: full SQLite (4525 tests) and Postgres (1816 tests) suites green. Same 1M-edge repro's per-batch time drops to ~90-160ms with a much flatter growth curve.
pdlug
added a commit
that referenced
this pull request
Jul 7, 2026
bulkCreate/bulkInsert), so this branch's local library build reflects the fix while investigating further bulk-loading optimizations.
pdlug
added a commit
that referenced
this pull request
Jul 7, 2026
Each bulkInsert() call gets its own transaction (runInWriteTransaction), while the bind-parameter-limit chunking that keeps individual INSERT statements within the driver's bound happens *inside* that call, independent of this outer size. A smaller outer batch just means more transactions/round-trips for the same total rows, not smaller/safer statements. Profiled on a synthetic 500k-row repro (post-#227): 2,000 (the old value) was consistently the slowest across two separate runs; anything >=10,000 was 25-30% faster, with the exact optimum noisy run-to-run on this shared dev machine (10k/20k/50k traded places for fastest between runs). 20,000 is a well-supported middle ground, not a precisely tuned peak — comfortably below both SQLite's (~3,640) and Postgres's (~7,281) internal per-statement chunk sizes' multiples, so this still issues multiple appropriately-sized statements per call rather than one enormous one. Verified: smoke suite still 100% row-count parity across all 4 engines.
Merged
pdlug
added a commit
that referenced
this pull request
Jul 7, 2026
Fresh full SF1 EC2 run reflecting PR #227 (edge-creation N+1 endpoint check) and the loader batch-size tuning: sqlite load time down ~1.85x, postgres down ~7x. Document that fix alongside the existing three. Also documents the concurrent-root-walk experiment for IS2/IS7: tried, measured on this same dedicated box, and reverted — neutral for SQLite (which serializes concurrent execute() calls anyway) and a mild regression for Postgres (this benchmark's Postgres runs over localhost Docker, so there's little real round-trip latency to overlap).
pdlug
added a commit
that referenced
this pull request
Jul 10, 2026
Each bulkInsert() call gets its own transaction (runInWriteTransaction), while the bind-parameter-limit chunking that keeps individual INSERT statements within the driver's bound happens *inside* that call, independent of this outer size. A smaller outer batch just means more transactions/round-trips for the same total rows, not smaller/safer statements. Profiled on a synthetic 500k-row repro (post-#227): 2,000 (the old value) was consistently the slowest across two separate runs; anything >=10,000 was 25-30% faster, with the exact optimum noisy run-to-run on this shared dev machine (10k/20k/50k traded places for fastest between runs). 20,000 is a well-supported middle ground, not a precisely tuned peak — comfortably below both SQLite's (~3,640) and Postgres's (~7,281) internal per-statement chunk sizes' multiples, so this still issues multiple appropriately-sized statements per call rather than one enormous one. Verified: smoke suite still 100% row-count parity across all 4 engines.
pdlug
added a commit
that referenced
this pull request
Jul 10, 2026
Fresh full SF1 EC2 run reflecting PR #227 (edge-creation N+1 endpoint check) and the loader batch-size tuning: sqlite load time down ~1.85x, postgres down ~7x. Document that fix alongside the existing three. Also documents the concurrent-root-walk experiment for IS2/IS7: tried, measured on this same dedicated box, and reverted — neutral for SQLite (which serializes concurrent execute() calls anyway) and a mild regression for Postgres (this benchmark's Postgres runs over localhost Docker, so there's little real round-trip latency to overlap).
pdlug
added a commit
that referenced
this pull request
Jul 13, 2026
…ormance claim
A third review found several places still described the parity gate
as row-count-only after it was upgraded to value-level digest
comparison, plus two unrelated stale claims:
- Runtime console output ("=== Row-count parity ==="), CLI option
doc, and code comments (cli.ts, request-plan.ts, snb-short-reads.ts,
run-sf1-ec2.ts) updated to describe both signals.
- README's parity section, "what every run writes" description, and
fairness-harness summary updated the same way.
- README's SF1 load-time numbers (75-80 min TypeGraph loads) predated
the N+1 endpoint-lookup fix (PR #227) that cut them to ~40/~12
minutes — updated to the current numbers, with a note that the
results doc is the canonical source if they drift again.
- Results doc and README both described Neo4j's current loader as the
retired batched `UNWIND ... IN TRANSACTIONS` approach; it's been the
offline `neo4j-admin database import` for a while — corrected in
both places.
- README now states plainly that this lane adapts LDBC SNB rather than
claiming full schema conformance: the schema flattens two
official-schema edges (City via IS_LOCATED_IN, Forum's moderator via
HAS_MODERATOR) into plain properties, identically across all four
engines. Query output fields and ordering are verified against the
official reference implementation; the schema-level simplification
is a separate, deliberate, and already-documented choice
(schema/snb-graph.ts's module doc) that the README should name
rather than let "official" imply exact conformance.
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.
Batches edge creation's endpoint-existence checks in
bulkCreate/bulkInsertinto onegetNodescall per distinct node kind referenced across the whole batch, instead of an individualgetNodeprobe per edge — mirroring the batched existence/uniqueness pre-check node creation already had viaprimeBatchValidationCaches(node creation was already batched; edge creation never got the equivalent treatment).Found while investigating why a real LDBC SNB SF1 bulk load (
packages/benchmarks, a separate in-flight PR) was far slower on TypeGraph/SQLite and TypeGraph/Postgres (~75-81 min) than on Neo4j (~4.4 min) or LadybugDB (~45s). A controlled 1M-row synthetic reproduction isolated the specific cause:hasCreatoredge-batch time grew from ~90ms to ~630ms per 2,000-row batch as the graph grew, while an equivalent node-only batch (no edges) stayed roughly flat. ReadingvalidateAndPrepareEdgeCreateshowed why: it callsbackend.getNode()individually for both thefromandtoendpoint of every edge, so a batch with mostly-unique endpoints (the common bulk-load case) does thousands of individual round trips instead of one batched fetch per kind.Changes
createEdgeBatchValidationBackendgainsseedEndpointRow(mirrorsseedNodeRowon the node side).primeEdgeBatchValidationCache: collects every distinct(kind, id)pair referenced across a batch'sfrom/toendpoints and issues onegetNodes()call per kind before the per-edge validation loop runs.prepareEdgeBatchCreates(mirrors the node side'sprepareBatchCreates) — bothexecuteEdgeCreateNoReturnBatchandexecuteEdgeCreateBatchnow call it instead of duplicating the same loop.