fix(pipeline): make the complexity pass independent of worker id order - #2079
Merged
Conversation
The complexity pass (transitive_loop_depth / recursive) walked Function and Method nodes in temp-id order and each node's CALLS targets in adjacency order. Under parallel extraction the temp ids come from one shared atomic counter, so id order is worker-scheduling order and differs run to run. The cycle guard flags whichever member of a mutual-recursion cycle the DFS enters first, so `recursive` flipped between the members of a cycle across otherwise identical multi-worker runs while the CALLS edge set stayed identical -- a violation of the MT-byte-identical invariant. Evidence: on a 66-file Python fixture of equal-sized mutual-recursion cycles (24 pairs + 6 triangles, adjacent in the size-ordered work queue) a probe saw 3 distinct outputs in 5 runs at 4 workers. The new regression test is RED on main 5/5 (a 4-worker run diverges from the single-threaded run on the first cycle every time) and GREEN with this change. Fix: order the traversal by content only -- seeds sorted by (qualified_name, file_path, start_line), and each node's CALLS targets sorted the same way before recursing, through one bump-stack scratch sized to the CALLS edge count (a single upfront allocation that bails the way the existing calloc path does). Semantics are unchanged: the DFS entry node of a cycle is the one flagged, as before; which member that is now depends on the inputs alone, identically for sequential and parallel indexing. SCC-based marking of every cycle member is out of scope here. Test: pipeline_complexity_props_independent_of_worker_order indexes the fixture once single-threaded and six times with CBM_WORKERS=4 through the same harness as the sequential/parallel parity test, collects every Function's complexity props sorted by qualified_name, and asserts all parallel runs are identical to the sequential run. Distilled from #1925 with co-author credit. Verification: build/c/test-runner pipeline complexity extraction (ASan/UBSan) 617 passed, 0 failed; revert-check RED 5/5 on main, GREEN with the fix; make -f Makefile.cbm lint-ci clean; scripts/check-no-test-skips.sh OK. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com> Co-authored-by: Zhiyu <zhiyuzhang001@gmail.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
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
The complexity pass (
transitive_loop_depth/recursive) walked Function and Method nodes in temp-id order and each node's CALLS targets in adjacency order. Under parallel extraction the temp ids come from one shared atomic counter, so id order is worker-scheduling order and differs run to run. The cycle guard flags whichever member of a mutual-recursion cycle the DFS enters first, sorecursiveflipped between the members of a cycle across otherwise identical multi-worker runs while the CALLS edge set stayed identical -- a violation of the MT-byte-identical invariant.Evidence
On a 66-file Python fixture of equal-sized mutual-recursion cycles (24 pairs + 6 triangles, adjacent in the size-ordered work queue) a probe saw 3 distinct outputs in 5 runs at 4 workers. The new regression test is RED on main 5/5 -- a 4-worker run diverges from the single-threaded run on the first cycle every time (
p00_a.pair00_a "recursive":falsevstrue) -- and GREEN with this change.Fix
Order the traversal by content only: seeds sorted by (qualified_name, file_path, start_line), and each node's CALLS targets sorted the same way before recursing, through one bump-stack scratch sized to the CALLS edge count (a single upfront allocation that bails the way the existing calloc path does). Semantics are unchanged: the DFS entry node of a cycle is the one flagged, as before; which member that is now depends on the inputs alone, identically for sequential and parallel indexing. SCC-based marking of every cycle member is out of scope here.
Test
pipeline_complexity_props_independent_of_worker_order(SUITE(pipeline), next to the sequential/parallel parity test) copiestests/fixtures/complexity_pass_cycle_orderinto a temp repo, indexes it once single-threaded and six times withCBM_WORKERS=4through the same harness, collects every Function's complexity props sorted by qualified_name, and asserts all parallel runs are identical to the sequential run. Deterministic, no timing.Verification
build/c/test-runner pipeline complexity extraction(ASan/UBSan): 617 passed, 0 failedmake -f Makefile.cbm lint-ci: cppcheck + clang-format + NOLINT check cleanscripts/check-no-test-skips.sh: OK; no rawfopen(, no SKIP/NOLINT, ASCII-only additionsDistilled from #1925 with co-author credit to @zhiyuzhang001-a11y.
Refs #1925