Skip to content

perf: components by a union-find over the edges - #63

Merged
petercorke merged 1 commit into
mainfrom
perf/components-union-find
Oct 13, 2026
Merged

petercorke merged 1 commit into
mainfrom
perf/components-union-find

Conversation

@petercorke

Copy link
Copy Markdown
Owner

The labelling of components (nc, component, samecomponent, Vertex.label) was O(V + E) but slow in practice: profiling at 10,000 vertices put 55% of the time in Edge.next, called once per incident edge from adjacent() (and again from predecessors() for a directed graph), each reading two properties.

It is now one pass over the edges with a union-find (path halving), reading the two ends of each edge directly:

  • direction is irrelevant by construction, so there is no directed special case (components are still weak);
  • the root of a component is its first vertex, so numbering the roots in vertex order gives exactly the same labels as before (checked against the old implementation and against a traversal written out in the tests);
  • each vertex's _label is written directly, not through Vertex.__setattr__.
before after
10,000 vertices, 30,000 edges 8.7 ms ~4 ms
100,000 vertices, 200,000 edges 136 ms ~40 ms

The "not recomputed when unchanged" test spied on Vertex.adjacent, which this does not call, so it now poisons the labels and checks that they survive until the graph changes. New tests: numbering follows the first vertex whatever the order of the edges, loops and parallel edges, removals and additions are changes, and agreement with an independent traversal on 30 random graphs of both kinds.

755 tests pass, mypy clean.

🤖 Generated with Claude Code

The labelling walked the neighbours of every vertex, which is a call to
Edge.next for every incident edge, and for a directed graph a second list of
predecessors.  Looking at the two ends of each edge once, with a union-find,
needs neither, ignores direction by construction, and gives the same labels:
the root of a component is its first vertex, so numbering the roots in the
order of the vertices keeps the numbering.  The labels are written directly
to the vertices, not through the coordinate hook.

10,000 vertices, 30,000 edges: 8.7 -> about 4 ms; 100,000 vertices, 200,000
edges: 136 -> about 40 ms.  The test of recomputation poisons the labels and
sees whether they survive, since it no longer calls adjacent(); new tests of
the numbering and of agreement with a traversal written out in the test.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 30 complexity · 2 duplication

Metric Results
Complexity 30
Duplication 2

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@petercorke
petercorke merged commit cdbc54a into main Oct 13, 2026
12 checks passed
@petercorke
petercorke deleted the perf/components-union-find branch October 13, 2026 00:19
@codecov

codecov Bot commented Oct 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.26%. Comparing base (041033f) to head (eb6974f).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #63      +/-   ##
==========================================
+ Coverage   97.24%   97.26%   +0.01%     
==========================================
  Files           3        3              
  Lines        1560     1570      +10     
==========================================
+ Hits         1517     1527      +10     
  Misses         43       43              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant