Elide redundant SPF runs in FlowPolicy with an exact-match memo - #8
Merged
Conversation
Profiling pass over max-flow and demand placement (9 operations x 5 topologies, sampled stacks + instrumented call counts). SPF dominates everything: ~92% of calc_max_flow, 60-80% of policy placement. Max-flow's own SPF usage is already minimal (one call per augmentation plus one termination proof), and two attempted SPF micro-optimizations measured as not worth it and were discarded (thread_local workspace reuse: +-0 within noise; 4-ary heap: 7% SLOWER than std::priority_queue at the small frontier sizes early-exit produces). The real finding came from fingerprinting SPF inputs per call: in EqualBalanced mode, 94% of the SPF calls in a place/rebalance cycle (196 calls, 12 unique inputs) and 48% in a single placement were exact input repeats. Two sources: equalization rounds that place nothing between queries, and rebalance remove+place round-trips that restore byte-identical residuals. Proportional mode measured 0% repeats. get_path_bundle now memoizes raw SPF results in EqualBalanced mode only, keyed by (src, dst, residual-awareness, residual content). Matching is exact: a FlowGraph state stamp -- a process-unique instance id plus a version bumped by every mutating method, which FlowGraph's encapsulation makes airtight -- is the O(1) fast path, and a full residual memcmp is the content path that catches the round-trips. min_flow participates via has_value() only: its value shapes the Proportional-only derived mask and never reaches SPF in EB mode (keying on the value cost most of the hit rate before this was noticed). The memo is bounded at ~512 KiB per policy (entry count adapts to graph size), MRU-ordered, and holds DAG copies whose cost is microseconds against the 100-300us SPF calls they replace. Cost gates and best_path_cost_ updates still run on every call. Measured against a same-conditions baseline (A/B/A protocol; noise floor +-2-5% established by benchmarking one build against itself): place/rebalance churn 31-70% faster across four topologies, single EB placement 6-26% faster, everything else within noise. SPF call counts confirm exact elision: the measured cycle drops from 196 SPF runs to its 12 unique inputs. All outputs bit-identical: a corpus SHA-256 over max-flow (all modes and summary artifacts), SPF DAGs, KSP, batch and policy outputs is unchanged before/after. 450 Python tests (3 new memo-staleness tests whose teeth were verified against a deliberately broken memo), 156 C++ tests, sanitizers clean, and NetGraph's 1217 tests pass against a wheel of this build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ddb3ee8b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review on #8 found a real hole: FlowGraph was implicitly copyable and copies shared uid_ and version_. Two copies that diverge after copying can reach equal versions with different residual content, and FlowPolicy's memo stamp fast path would then serve a DAG cached for the other object's state without ever reaching the content memcmp. Copy and move operations now assign a fresh process-unique uid (and version 0). Moves too: a moved-from FlowGraph remains valid and mutable, so sharing its uid would reopen the same hole; the cost is one cache miss after a move. Two C++ tests pin the invariant -- uids differ across copy/move construct and assign, and equal-version divergent copies compare unequal -- and both were verified to fail against a sabotaged shared-uid copy constructor. 450 Python + 158 C++ tests pass, sanitizers clean, corpus hash unchanged. Reported by Codex review on #8. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
A methodical profiling pass over max-flow and demand placement, with every candidate
measured before being kept — including two that measured as not worth it and were
discarded.
Method
9 operations × 5 topologies (Clos ×2, dispersed-cost WAN ×2, tiered parallel-edge),
median-of-reps harness with a measured ±2–5% noise floor (established by benchmarking one
build against itself), sampled C++ stacks, instrumented SPF/placement call counts, and a
corpus SHA-256 over max-flow (all modes + summary artifacts), SPF DAGs, KSP, batch, and
policy outputs as a bit-identity gate.
What profiling showed
SPF is the program. ~92% of
calc_max_flow, 60–80% of policy placement. Max-flow'sown SPF usage is already minimal — one call per augmentation plus one termination proof.
Two negative results, both reverted:
std::priority_queueThe real finding: 94% of EB-mode SPF calls are exact input repeats
Fingerprinting every SPF call's full input (src, dst, residuals, masks) showed a measured
EqualBalanced place/rebalance cycle ran 196 SPF calls over just 12 unique inputs.
Sources: equalization rounds that place nothing between queries, and rebalance
remove+place round-trips that restore byte-identical residuals. Proportional mode: 0%
repeats.
The fix: an exact-match memo in
get_path_bundle, EB-onlyFlowGraphstate stamp(process-unique uid + mutation-bumped version, airtight because
FlowStateis fullyencapsulated) as the O(1) fast path; full residual
memcmpas the content path thatcatches the round-trips.
min_flowkeys onhas_value()only — its value shapes the Proportional-only derivedmask and never reaches SPF in EB mode. (Keying on the value cost most of the hit rate
before this was noticed; call counts exposed it.)
best_path_cost_updates still run on every call — they depend onmutable policy state.
Results (A/B/A, same-conditions baseline)
Call counts confirm exact elision: the measured cycle drops from 196 SPF runs to its 12
unique inputs — the theoretical minimum.
Validation
Corpus SHA-256 unchanged through every iteration · 450 Python tests (3 new
memo-staleness tests, teeth verified by deliberately breaking the memo key and watching
them fail) · 156 C++ tests · ASan/UBSan (fatal) clean · downstream NetGraph: 1217
tests pass against a wheel of this build.
🤖 Generated with Claude Code