perf: Faster singleton SiblingSubgraph construction (#1654)

Wraps the topoConvexChecker used by `SiblingSubgraph` in a `OnceCell` 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. See benchmark results: ``` $ critcmp before after -f '.*subgraph' group after before ----- ----- ------ singleton_subgraph/10 1.00 3.0±0.08µs ? ?/sec 2.71 8.1±0.18µs ? ?/sec singleton_subgraph/100 1.00 4.8±0.07µs ? ?/sec 9.81 47.4±1.16µs ? ?/sec singleton_subgraph/1000 1.00 23.2±1.86µs ? ?/sec 18.94 439.3±17.04µs ? ?/sec multinode_subgraph/10 1.01 17.9±0.25µs ? ?/sec 1.00 17.7±0.29µs ? ?/sec multinode_subgraph/100 1.01 170.0±3.72µs ? ?/sec 1.00 168.5±3.04µs ? ?/sec multinode_subgraph/1000 1.02 2.4±0.02ms ? ?/sec 1.00 2.3±0.04ms ? ?/sec ``` `singleton_subgraph` creates a single-node subgraph in a region with around `k * 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 to `cargo bench`.
CQCL · Nov 13, 2024 · e63878f · e63878f
1 parent 0c430d1
commit e63878f
Show file tree

Hide file tree

Showing 11 changed files with 252 additions and 135 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -81,7 +81,7 @@ repos:
       - id: cargo-clippy
         name: cargo clippy
         description: Run clippy lints with `cargo clippy`.
-        entry: cargo clippy --all-features --workspace -- -D warnings
+        entry: cargo clippy --all-targets --all-features --workspace -- -D warnings
         language: system
         files: \.rs$
         pass_filenames: false

diff --git a/hugr-cli/Cargo.toml b/hugr-cli/Cargo.toml
@@ -12,6 +12,9 @@ description = "Compiler passes for Quantinuum's HUGR"
 keywords = ["Quantum", "Quantinuum"]
 categories = ["compilers"]
 
+[lib]
+bench = false
+
 [dependencies]
 clap = { workspace = true, features = ["derive"] }
 clap-verbosity-flag.workspace = true
@@ -42,3 +45,4 @@ rstest.workspace = true
 name = "hugr"
 path = "src/main.rs"
 doc = false
+bench = false
diff --git a/hugr-core/Cargo.toml b/hugr-core/Cargo.toml
@@ -21,6 +21,9 @@ extension_inference = []
 declarative = ["serde_yaml"]
 model_unstable = ["hugr-model"]
 
+[lib]
+bench = false
+
 [[test]]
 name = "model"
 required-features = ["model_unstable"]

diff --git a/hugr-core/src/hugr/views/sibling_subgraph.rs b/hugr-core/src/hugr/views/sibling_subgraph.rs
@@ -9,6 +9,7 @@
 //! while the former provide views for subgraphs within a single level of the
 //! hierarchy.
 
+use std::cell::OnceCell;
 use std::collections::HashSet;
 use std::mem;
 
@@ -453,15 +454,24 @@ fn combine_in_out<'a>(
 ///
 /// This can be used when constructing multiple sibling subgraphs to speed up
 /// convexity checking.
-pub struct TopoConvexChecker<'g, Base: 'g + HugrView>(
-    portgraph::algorithms::TopoConvexChecker<Base::Portgraph<'g>>,
-);
+pub struct TopoConvexChecker<'g, Base: 'g + HugrView> {
+    base: &'g Base,
+    checker: OnceCell<portgraph::algorithms::TopoConvexChecker<Base::Portgraph<'g>>>,
+}
 
 impl<'g, Base: HugrView> TopoConvexChecker<'g, Base> {
     /// Create a new convexity checker.
     pub fn new(base: &'g Base) -> Self {
-        let pg = base.portgraph();
-        Self(portgraph::algorithms::TopoConvexChecker::new(pg))
+        Self {
+            base,
+            checker: OnceCell::new(),
+        }
+    }
+
+    /// Returns the portgraph convexity checker, initializing it if necessary.
+    fn get_checker(&self) -> &portgraph::algorithms::TopoConvexChecker<Base::Portgraph<'g>> {
+        self.checker
+            .get_or_init(|| portgraph::algorithms::TopoConvexChecker::new(self.base.portgraph()))
     }
 }
 
@@ -472,7 +482,13 @@ impl<'g, Base: HugrView> ConvexChecker for TopoConvexChecker<'g, Base> {
         inputs: impl IntoIterator<Item = portgraph::PortIndex>,
         outputs: impl IntoIterator<Item = portgraph::PortIndex>,
     ) -> bool {
-        self.0.is_convex(nodes, inputs, outputs)
+        let mut nodes = nodes.into_iter().multipeek();
+        // If the node iterator contains less than two nodes, the subgraph is
+        // trivially convex.
+        if nodes.peek().is_none() || nodes.peek().is_none() {
+            return true;
+        };
+        self.get_checker().is_convex(nodes, inputs, outputs)
     }
 }
 

diff --git a/hugr-model/Cargo.toml b/hugr-model/Cargo.toml
@@ -12,6 +12,9 @@ homepage.workspace = true
 repository.workspace = true
 license.workspace = true
 
+[lib]
+bench = false
+
 [dependencies]
 bumpalo = { workspace = true, features = ["collections"] }
 capnp = "0.20.1"

diff --git a/hugr-passes/Cargo.toml b/hugr-passes/Cargo.toml
@@ -12,6 +12,9 @@ description = "Compiler passes for Quantinuum's HUGR"
 keywords = ["Quantum", "Quantinuum"]
 categories = ["compilers"]
 
+[lib]
+bench = false
+
 [dependencies]
 hugr-core = { path = "../hugr-core", version = "0.13.3" }
 itertools = { workspace = true }

diff --git a/hugr/benches/bench_main.rs b/hugr/benches/bench_main.rs
@@ -7,5 +7,6 @@ use criterion::criterion_main;
 
 criterion_main! {
     benchmarks::hugr::benches,
+    benchmarks::subgraph::benches,
     benchmarks::types::benches,
 }
diff --git a/hugr/benches/benchmarks/hugr.rs b/hugr/benches/benchmarks/hugr.rs
@@ -1,56 +1,13 @@
 #![allow(clippy::unit_arg)] // Required for black_box uses
 
+pub mod examples;
+
 use criterion::{black_box, criterion_group, AxisScale, BenchmarkId, Criterion, PlotConfiguration};
-use hugr::builder::{
-    BuildError, CFGBuilder, Container, DFGBuilder, Dataflow, DataflowHugr, DataflowSubContainer,
-    HugrBuilder, ModuleBuilder,
-};
-use hugr::extension::prelude::{BOOL_T, QB_T, USIZE_T};
-use hugr::extension::PRELUDE_REGISTRY;
-use hugr::ops::OpName;
+#[allow(unused)]
 use hugr::std_extensions::arithmetic::float_ops::FLOAT_OPS_REGISTRY;
-use hugr::std_extensions::arithmetic::float_types::FLOAT64_TYPE;
-use hugr::types::Signature;
-use hugr::{type_row, Extension, Hugr};
-use lazy_static::lazy_static;
-pub fn simple_dfg_hugr() -> Hugr {
-    let dfg_builder =
-        DFGBuilder::new(Signature::new(type_row![BOOL_T], type_row![BOOL_T])).unwrap();
-    let [i1] = dfg_builder.input_wires_arr();
-    dfg_builder.finish_prelude_hugr_with_outputs([i1]).unwrap()
-}
-
-pub fn simple_cfg_builder<T: AsMut<Hugr> + AsRef<Hugr>>(
-    cfg_builder: &mut CFGBuilder<T>,
-) -> Result<(), BuildError> {
-    let sum2_variants = vec![type_row![USIZE_T], type_row![USIZE_T]];
-    let mut entry_b = cfg_builder.entry_builder(sum2_variants.clone(), type_row![])?;
-    let entry = {
-        let [inw] = entry_b.input_wires_arr();
-
-        let sum = entry_b.make_sum(1, sum2_variants, [inw])?;
-        entry_b.finish_with_outputs(sum, [])?
-    };
-    let mut middle_b = cfg_builder
-        .simple_block_builder(Signature::new(type_row![USIZE_T], type_row![USIZE_T]), 1)?;
-    let middle = {
-        let c = middle_b.add_load_const(hugr::ops::Value::unary_unit_sum());
-        let [inw] = middle_b.input_wires_arr();
-        middle_b.finish_with_outputs(c, [inw])?
-    };
-    let exit = cfg_builder.exit_block();
-    cfg_builder.branch(&entry, 0, &middle)?;
-    cfg_builder.branch(&middle, 0, &exit)?;
-    cfg_builder.branch(&entry, 1, &exit)?;
-    Ok(())
-}
+use hugr::Hugr;
 
-pub fn simple_cfg_hugr() -> Hugr {
-    let mut cfg_builder =
-        CFGBuilder::new(Signature::new(type_row![USIZE_T], type_row![USIZE_T])).unwrap();
-    simple_cfg_builder(&mut cfg_builder).unwrap();
-    cfg_builder.finish_prelude_hugr().unwrap()
-}
+pub use examples::{circuit, simple_cfg_hugr, simple_dfg_hugr};
 
 trait Serializer {
     fn serialize(&self, hugr: &Hugr) -> Vec<u8>;
@@ -90,83 +47,6 @@ fn roundtrip(hugr: &Hugr, serializer: impl Serializer) -> Hugr {
     serializer.deserialize(&bytes)
 }
 
-lazy_static! {
-    static ref QUANTUM_EXT: Extension = {
-        let mut extension = Extension::new(
-            "bench.quantum".try_into().unwrap(),
-            hugr::extension::Version::new(0, 0, 0),
-        );
-
-        extension
-            .add_op(
-                OpName::new_inline("H"),
-                "".into(),
-                Signature::new_endo(QB_T),
-            )
-            .unwrap();
-        extension
-            .add_op(
-                OpName::new_inline("Rz"),
-                "".into(),
-                Signature::new(type_row![QB_T, FLOAT64_TYPE], type_row![QB_T]),
-            )
-            .unwrap();
-
-        extension
-            .add_op(
-                OpName::new_inline("CX"),
-                "".into(),
-                Signature::new_endo(type_row![QB_T, QB_T]),
-            )
-            .unwrap();
-        extension
-    };
-}
-
-pub fn circuit(layers: usize) -> Hugr {
-    let h_gate = QUANTUM_EXT
-        .instantiate_extension_op("H", [], &PRELUDE_REGISTRY)
-        .unwrap();
-    let cx_gate = QUANTUM_EXT
-        .instantiate_extension_op("CX", [], &PRELUDE_REGISTRY)
-        .unwrap();
-    // let rz = QUANTUM_EXT
-    //     .instantiate_extension_op("Rz", [], &FLOAT_OPS_REGISTRY)
-    //     .unwrap();
-    let signature =
-        Signature::new_endo(type_row![QB_T, QB_T]).with_extension_delta(QUANTUM_EXT.name().clone());
-    let mut module_builder = ModuleBuilder::new();
-    let mut f_build = module_builder.define_function("main", signature).unwrap();
-
-    let wires: Vec<_> = f_build.input_wires().collect();
-
-    let mut linear = f_build.as_circuit(wires);
-
-    for _ in 0..layers {
-        linear
-            .append(h_gate.clone(), [0])
-            .unwrap()
-            .append(cx_gate.clone(), [0, 1])
-            .unwrap()
-            .append(cx_gate.clone(), [1, 0])
-            .unwrap();
-
-        // TODO: Currently left out because we can not represent constants in the model
-        // let angle = linear.add_constant(ConstF64::new(0.5));
-        // linear
-        //     .append_and_consume(
-        //         rz.clone(),
-        //         [CircuitUnit::Linear(0), CircuitUnit::Wire(angle)],
-        //     )
-        //     .unwrap();
-    }
-
-    let outs = linear.finish();
-    f_build.finish_with_outputs(outs).unwrap();
-
-    module_builder.finish_hugr(&FLOAT_OPS_REGISTRY).unwrap()
-}
-
 fn bench_builder(c: &mut Criterion) {
     let mut group = c.benchmark_group("builder");
     group.plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic));
@@ -187,7 +67,7 @@ fn bench_serialization(c: &mut Criterion) {
     group.plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic));
     for size in [0, 1, 10, 100, 1000].iter() {
         group.bench_with_input(BenchmarkId::from_parameter(size), size, |b, &size| {
-            let h = circuit(size);
+            let h = circuit(size).0;
             b.iter(|| {
                 black_box(roundtrip(&h, JsonSer));
             });
@@ -199,7 +79,7 @@ fn bench_serialization(c: &mut Criterion) {
     group.plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic));
     for size in [0, 1, 10, 100, 1000].iter() {
         group.bench_with_input(BenchmarkId::from_parameter(size), size, |b, &size| {
-            let h = circuit(size);
+            let h = circuit(size).0;
             b.iter(|| {
                 black_box(JsonSer.serialize(&h));
             });
@@ -213,7 +93,7 @@ fn bench_serialization(c: &mut Criterion) {
         group.plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic));
         for size in [0, 1, 10, 100, 1000].iter() {
             group.bench_with_input(BenchmarkId::from_parameter(size), size, |b, &size| {
-                let h = circuit(size);
+                let h = circuit(size).0;
                 b.iter(|| {
                     black_box(roundtrip(&h, CapnpSer));
                 });