-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf: Faster singleton SiblingSubgraph construction #1654
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1654 +/- ##
=======================================
Coverage 85.50% 85.51%
=======================================
Files 136 136
Lines 25252 25264 +12
Branches 22164 22176 +12
=======================================
+ Hits 21592 21604 +12
Misses 2456 2456
Partials 1204 1204
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
This PR contains breaking changes to the public Rust API. cargo-semver-checks summary
|
Uh, looks like I introduced a breaking change -.- |
Awk. I think we can put it in a Mutex to restore unbrokenness? |
Complementary improvement to #1654. Creating a k-node subgraph in an n-node graph should ideally be `O(k)`. However, due to CQCL/portgraph#155 this ends up being `O(n)`. For `k=1`, this results in a linear cost overhead. This PR adds a special case (written by @doug-q) that completely skips the unnecessary checks. ``` group before from_node ----- ------ --------- multinode_subgraph/10 1.01 17.7±0.26µs ? ?/sec 1.00 17.5±0.23µs ? ?/sec multinode_subgraph/100 1.00 169.1±11.34µs ? ?/sec 1.00 168.8±4.37µs ? ?/sec multinode_subgraph/1000 1.01 2.3±0.46ms ? ?/sec 1.00 2.3±0.34ms ? ?/sec singleton_subgraph/10 12.26 3.0±0.06µs ? ?/sec 1.00 245.6±21.24ns ? ?/sec singleton_subgraph/100 20.01 4.7±0.06µs ? ?/sec 1.00 234.4±6.50ns ? ?/sec singleton_subgraph/1000 93.34 22.0±0.25µs ? ?/sec 1.00 235.6±4.93ns ? ?/sec ``` --------- Co-authored-by: Douglas Wilson <141026920+doug-q@users.noreply.github.com>
Wraps the topoConvexChecker used by
SiblingSubgraph
in aOnceCell
so we can delay initialization until it's needed.This lets us avoid traversing the graph (and computing a topological order) when creating sibling subgraphs with zero or one nodes.
This partially helps with #1652.
See benchmark results:
singleton_subgraph
creates a single-node subgraph in a region with aroundk * 3
nodes.multinode_subgraph
creates a subgraph with 1/3rd of the nodes for the same region.This PR is quite noisy since it's adding those two new benchmarks, and tidying up the files in the process.
drive-by: Add
bench = false
for all the targets in the workspace. Otherwise, the auto-added test harness threw errors when passing criterion flags tocargo bench
.