Skip to content

perf(traversal): adaptive mode switch, limit pushdown, persisted CSR - #544

Open
azimafroozeh wants to merge 2 commits into
ModernRelay:mainfrom
azimafroozeh:traversal-refactor
Open

perf(traversal): adaptive mode switch, limit pushdown, persisted CSR#544
azimafroozeh wants to merge 2 commits into
ModernRelay:mainfrom
azimafroozeh:traversal-refactor

Conversation

@azimafroozeh

@azimafroozeh azimafroozeh commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

What & why

Closes #533. The traversal-mode chooser ran once before hop 1, so a single-seed high-fanout traversal (frontier 1 at dispatch, 92,889 by hop 4 on the reported shape) committed to the indexed path and stayed there. This PR reworks the Expand execution path around that bug and ships four improvements; every measured result is collected in the results section below.

  1. Adaptive mode switch: the cost decision reruns at every indexed hop with the observed frontier (projected forward by the observed growth ratio; the frontier ceiling is now an execution bound, and undirected traversals are priced at their true two probes per hop). An outgrown traversal switches to CSR mid-flight, carrying its BFS state, no restart.
  2. Limit pushdown: an unordered, aggregate-free limit on a final filterless Expand stops the traversal at the cap, with an uncapped rerun if hydration drops below it.
  3. Unified BFS core: the duplicated indexed and CSR executors become one loop over a pluggable neighbor source; every traversal contract (emission, dedup, hop gating, self-edge, cross-type) is written once, and the mid-flight switch is just a source swap. Pure-path overhead is within noise.
  4. Persisted graph index: optimize writes the CSR/CSC plus dictionaries to __graph_index/csr-current.bin (binary sections, per-edge-dataset identity stamps, payload sha256); cold traversal builds load it with one GET instead of scanning every edge table.

Measured results, before this PR vs after

Environment: local NVMe, release build, medians of warm runs, both legs from the same instrument on the same machine; the before leg is the pristine merge base. The instrument is a drop-in example (the #533 reproduction extended with scale arguments), not part of this PR. Query legs use the #533 graph shape: 388k nodes, 2,519,918 edges, seed degree 238, per-hop frontier 1 / 238 / 5,418 / 92,889 against the 1,024 ceiling.

What the engine does on its own (auto mode):

Scenario Before this PR After this PR Speedup What changed
The issue's query: 4 hops, undirected, limit 100 750.0 ms 8.6 ms 87x limit pushdown stops the traversal at hop 1
Same query, uncapped (250,442 results) ~750 ms (auto tracked forced-indexed, 753.8 ms) 247.8 ms 3.0x the mid-flight switch moves auto onto the CSR path
Cold CSR acquisition, 25M-edge store 1.95 s (edge-table scan) 229 ms (one GET of the 240 MiB artifact) 8.5x persisted graph index

Sanity: results are identical in every mode and every scenario (100 and 250,442 rows respectively), the after-auto uncapped time matches forced CSR (247.8 vs 247.9 ms), and the forced modes themselves are unchanged within noise, so the wins come from better decisions, not a changed engine core.

Format decision behind the artifact (not a before/after: upstream never had an artifact): both encodings measured over identical in-memory data at 100M edges (1,060 MiB raw). Binary ships because it decodes 1.9x faster than JSON+base64 (0.61 s vs 1.17 s, paid on every cold load) and is 21% smaller (1,061 vs 1,339 MiB, paid on every cold GET). A digest-free floor variant (0.30 s) was measured and rejected: about half of binary decode is the sha256, kept unconditionally because raw arrays have no syntax to fail on and a flipped byte would otherwise become silently wrong topology.

Backing issue / RFC

Checklist

  • Change is focused (one rework of the Expand execution path; the pieces share the unified core)
  • Tests added/updated (traversal_adaptive suite: 8, incl. switch equality directed/undirected, min-hops across the switch, capped-limit subset validity, artifact write/corruption/staleness; indexed/CSR equivalence battery 13 now exercises the unified core; chooser units 15; persist units 22 incl. the crafted-artifact class)
  • Public docs updated (docs/user/queries/index.md, docs/user/reference/constants.md, docs/user/operations/maintenance.md, docs/user/concepts/storage.md, docs/dev/invariants.md)
  • Reviewed against docs/dev/invariants.md: no Hard Invariant weakened; __graph_index/ is derived, regenerable, optimize-only-written, never authoritative

Local verification

  • cargo test --workspace --no-fail-fast: 80 suites ok; the one failure is the pre-existing local-environment external_blob_file_policy_rejects_special_files
  • cargo clippy --workspace --all-targets clean; cargo fmt --all applied
  • vocabulary guard: user-docs and openapi clean; rust-string inventory updated for the executor rename and persist.rs (rows reviewed)

Notes for reviewers

  • Semantics change: unordered, aggregate-free limit n now returns an arbitrary valid n-subset instead of the previous deterministic slice (count exact; ordered/aggregated queries untouched). Multi-seed CSR row order also changed (hop-major); unordered order is now explicitly uncontracted. Both documented.
  • __graph_index/ is a new top-level store prefix: one derived object, optimize is the sole writer, queries only read, every failure or staleness falls open to the in-memory scan build; no GC yet.
  • The loader validates beyond the digest (offsets cover the keyed dictionary, targets bounded by the opposite one, allocation clamped, every stamped edge must carry its adjacency), so an internally inconsistent artifact is rejected fail-open, never a panic or dispatch error.
  • New byte primitives read_bytes_if_exists_bounded / write_bytes on both StorageAdapter traits; forbidden_apis registers the persist write as PhysicalOnly (derived, regenerable, never graph-visible).
  • The artifact's stamps must stay in lockstep with the in-memory graph-index cache key (cross-referenced in both files); on e_tag-less local FS it inherits that key's documented branch-ref ABA residual.
  • Known deferred costs, follow-up with a shared fix (header-first ranged reads): a stale artifact is rejected only after a full-object GET, and a scoped load caches the full-catalog index under its scoped key.

Greptile Summary

The PR unifies indexed and CSR traversal execution while adding adaptive strategy switching, safe limit pushdown, and persisted graph-index loading.

  • Re-evaluates traversal cost at each indexed hop and can continue from existing BFS state using CSR.
  • Pushes eligible unordered limits into a final filterless Expand, with an uncapped fallback after hydration under-fill.
  • Persists validated CSR/CSC artifacts during optimize and loads them through the runtime cache.
  • Adds binary storage primitives, lifecycle coverage, traversal tests, instrumentation, and documentation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
crates/omnigraph/src/exec/query.rs Unifies traversal execution, adds per-hop adaptive switching, and safely gates final-Expand limit pushdown.
crates/omnigraph/src/graph_index/persist.rs Introduces the checksummed persisted graph-index format with identity and structural validation.
crates/omnigraph/src/runtime_cache.rs Integrates persisted artifact loading into graph-index cache construction with build fallback.
crates/omnigraph/src/db/omnigraph/optimize.rs Persists a refreshed derived graph index after productive edge-table optimization.
crates/omnigraph-storage/src/lib.rs Adds bounded binary reads and binary writes to the storage adapter contract.
crates/omnigraph/tests/traversal_adaptive.rs Exercises adaptive switching, capped traversal behavior, and persisted artifact validation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Q[Execute query] --> D{Initial traversal mode}
    D -->|CSR| L[Load persisted graph index]
    L -->|Valid| C[Unified BFS with CSR source]
    L -->|Missing, stale, or invalid| B[Build graph index in memory]
    B --> C
    D -->|Indexed| I[Unified BFS with indexed source]
    I --> H{Next-hop frontier exceeds adaptive threshold?}
    H -->|No| I
    H -->|Yes| S[Translate BFS state into CSR dictionaries]
    S --> C
    I --> E[Hydrate emitted destinations]
    C --> E
    E --> U{Capped result under-filled?}
    U -->|Yes| R[Rerun Expand uncapped]
    U -->|No| P[Project and apply final limit]
    R --> P
    O[Optimize edge datasets] --> A[Encode and persist CSR/CSC artifact]
    A --> L
Loading

Reviews (2): Last reviewed commit: "vocab" | Re-trigger Greptile

Context used:

@azimafroozeh
azimafroozeh marked this pull request as ready for review August 22, 2026 18:24
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

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.

performance: traversal mode selection does not adapt as a multi-hop frontier grows

1 participant