perf: Avoid cloning signatures for simple port checks by aborgna-q · Pull Request #3149 · Quantinuum/hugr

aborgna-q · 2026-07-10T13:02:06Z

Avoids creating a whole signature object when calling OpType::value_port_count and OpType::port_kind.

Adds an internal ValuePortOp trait to dispatch the call to each optype variant.

Part of #1551

codspeed-hq · 2026-07-10T13:10:47Z

Merging this PR will improve performance by 9.63%

⚠️

Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 9 improved benchmarks
✅ 36 untouched benchmarks

Performance Changes

	Benchmark	`BASE`	`HEAD`	Efficiency
⚡	`simple_cfg`	383.3 µs	328 µs	+16.87%
⚡	`serialization/t_factory/capnp/with_extensions/encode`	11.7 ms	10.2 ms	+14.35%
⚡	`serialization/t_factory/capnp/without_extensions/encode`	10.8 ms	9.6 ms	+12.25%
⚡	`simple_dfg`	127.3 µs	116.5 µs	+9.28%
⚡	`fewnode_subgraph[1000]`	20 ms	18.6 ms	+7.22%
⚡	`serialization/simple_cfg/capnp/with_extensions/encode`	328.4 µs	307.1 µs	+6.93%
⚡	`serialization/t_factory/capnp/without_extensions/decode`	14.2 ms	13.3 ms	+6.9%
⚡	`fewnode_subgraph[100]`	2.1 ms	2 ms	+6.84%
⚡	`serialization/simple_cfg/capnp/without_extensions/encode`	309.5 µs	290.5 µs	+6.57%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing ab/envelope-perf-2 (238ba09) with main (e0540a1)}

codecov · 2026-07-10T13:12:12Z

Codecov Report

❌ Patch coverage is 93.50000% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.61%. Comparing base (e609c66) to head (238ba09).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
hugr-core/src/ops.rs	95.18%	3 Missing and 1 partial ⚠️
hugr-core/src/ops/sum.rs	81.25%	2 Missing and 1 partial ⚠️
hugr-core/src/ops/controlflow.rs	93.54%	2 Missing ⚠️
hugr-core/src/ops/dataflow.rs	96.55%	2 Missing ⚠️
hugr-core/src/hugr/views.rs	66.66%	1 Missing ⚠️
hugr-core/src/hugr/views/sibling_subgraph.rs	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3149      +/-   ##
==========================================
+ Coverage   81.55%   81.61%   +0.06%     
==========================================
  Files         242      242              
  Lines       47373    47649     +276     
  Branches    40970    41246     +276     
==========================================
+ Hits        38633    38888     +255     
- Misses       6721     6737      +16     
- Partials     2019     2024       +5

Flag	Coverage Δ
python	`89.39% <ø> (ø)`
rust	`80.40% <93.50%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

aborgna-q · 2026-07-13T16:48:35Z

@codspeedbot explain why this is faster

codspeed-hq · 2026-07-13T16:50:22Z

@codspeedbot explain why this is faster

Why this PR is faster

CodSpeed measures a +14.79% overall improvement, driven by three benchmarks:

Benchmark	Base → Head	Change
`builder::simple_cfg`	211.7 µs → 177.9 µs	+18.98%
`serialization/t_factory/capnp/without_extensions/encode`	5.2 ms → 4.6 ms	+14.41%
`serialization/t_factory/capnp/with_extensions/encode`	6.9 ms → 6.2 ms	+11.1%

Root cause: eliminating whole-`Signature` allocations on the hot path

Previously, OpType::value_port_count and OpType::port_kind went through dataflow_signature(). For op variants whose signature is synthesized (e.g. DataflowBlock, Case), this built a fresh Signature object on every call — allocating and deep-cloning the TypeRows and Terms inside it — just to read a port count or a single port's edge kind.

This PR introduces the internal ValuePortOp trait (hugr-core/src/ops.rs), dispatched per OpType variant via enum_dispatch, so these queries are answered without materializing the full signature:

Variants with a cached signature simply expose it (value_port_signature), so counting ports never allocates.
value_port_kind clones only the single requested port type instead of the entire signature.
Variants with no value ports get a trivial "zero ports" implementation.

Where the time went

These port queries are extremely hot: port_kind / value_port_count are called throughout HUGR validation (which is ~40% of simple_cfg) and during serialization encoding. In the head flamegraph for simple_cfg, the residual cost of these helpers is now dominated by the unavoidable work — the remaining allocation/clone traffic shows up as Term::clone (7.5% total), its drop_glue (4.3%), and malloc/free churn. Removing the per-call full-signature construction (and the matching allocate + clone + drop of every TypeRow/Term it contained) is what cuts CPU cycles across all three benchmarks.

In short: the speedup comes from not building and immediately discarding a complete Signature for what are really trivial "how many value ports?" / "what kind is this one port?" lookups.

Note: CodSpeed flagged that the base and head runs executed on slightly different CI hardware (EPYC 9V74 vs 7763). In Simulation mode this can shift the estimate a little, but the change here removes real allocation and cloning work, so the improvement is expected to be genuine rather than an artifact of runner variability.

Summary

Warming up the performance wizard...
Handling comment — Comment handling finished successfully

_{Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.}

acl-cqc · 2026-07-14T08:14:13Z

 }
+
+impl ValuePortOp for Call {
+    fn value_port_signature(&self) -> Option<&Signature> {


Does this mean that if you pass in the static-input port you'll get back None as the edge kind?

Oh, sorry, that might be deliberate....

Yes, this is trait replaces querying the signature for info on the value ports. Other ports are already tracked directly in the public OpTrait implementation.

Ideally the new ValuePortOp trait methods would be part of OpTrait, but I wanted to avoid breaking changes.

I can try unifying things more, or leave a TODO to seal OpTrait and add the methods there.

I guess adding default-implemented methods to OpTrait wouldn't be a breaking change 🤔

acl-cqc

Main concern here is just about duplicating logic in two places (the new method impls, and the old ones that constructs a Signature)
Shame that this is internal to hugr-core; I don't see why external clients (e.g. some tket2 passes!) wouldn't benefit from the same
Makes me think that we should make something like this be the main way to query a node - I think that means some mix of
- Deprecating signature()
- Changing the type fn signature(&self) -> impl ValuePortThing (...maybe instead of the methods on the op)
- Making Signature implement this trait too, and fn signature just builds it as a cache of what the op methods are returning
Note that there is HugrView::{in_,out_,}value_types as well, that calls the "expensive" (sometimes) Signature...maybe they just need updating

aborgna-q · 2026-07-16T14:02:25Z

Refactored the change, adding the new methods to OpTrait instead.

aborgna-q · 2026-07-31T08:51:21Z

+    /// Returns the type of a value port.
+    ///
+    /// Implementations may override this to avoid constructing a complete
+    /// [`Signature`] when only one port type is needed.
+    fn value_port_type(&self, port: Port) -> Option<Type> {
+        self.dataflow_signature()?.port_type(port).cloned()
+    }
+
+    /// Returns the number of value ports in one direction.
+    ///
+    /// Implementations may override this to avoid constructing a complete
+    /// [`Signature`] when only its size is needed.
+    fn value_port_count(&self, dir: Direction) -> usize {
+        self.dataflow_signature()
+            .map_or(0, |signature| signature.port_count(dir))
+    }


The new OpTrait methods are here.
They default to querying the signature so it's not a breaking change.

aborgna-q · 2026-07-31T09:16:40Z

+                let op = hugr.get_optype(n);
+                op.value_input_type(p).expect("must be dataflow edge")


I'm wondering if we should have a shorthand for this on HugrView.

For get_optype(n).value_(in/out)put_type, for value_(in/out)put_type.expect, or all three? Not opposed to any of those

Or (Node) -> impl Iterator<Item=Type>? (+direction, or *2 for in/out)

Looking more into it, I think it's fine to leave the the port-specific type getters in OpType.

This PR already adds HugrView::value_types(node, dir) -> Iterator<(Port, Type)> and in_/out_ variants. That should be enough to simplify relevant calls.

Hang on....HugrView already defines

hugr/hugr-core/src/hugr/views.rs

Line 496 in e609c66

fn in_value_types(&self, node: Self::Node) -> impl Iterator<Item = (IncomingPort, Type)> {

....that's not new in this PR ? [EDIT: ah yes, but I see you have reimplemented those on top of your new methods 👍 ]

aborgna-q · 2026-08-03T13:26:53Z

Main concern here is just about duplicating logic in two places (the new method impls, and the old ones that constructs a Signature)

Done, we're using the existing traits now

Shame that this is internal to hugr-core; I don't see why external clients (e.g. some tket2 passes!) wouldn't benefit from the same

It's now in the public trait

Makes me think that we should make something like this be the main way to query a node - I think that means some mix of

Deprecating signature()

Changing the type fn signature(&self) -> impl ValuePortThing (...maybe instead of the methods on the op)

Making Signature implement this trait too, and fn signature just builds it as a cache of what the op methods are returning

Could be interesting, I'll track it in a separate issue since it would be a breaking change.

Note that there is HugrView::{in_,out_,}value_types as well, that calls the "expensive" (sometimes) Signature...maybe they just need updating

Updated!

acl-cqc

Like this, good work @aborgna-q :)

Happy to approve like this but what do you think about having an iterator over (in/out-Port, Type) or even just Type for a node/direction? (value_ports only returns ports)

Could be on OpType instead of the method taking a in/out-Port, or as well, or I suppose on the Hugr(View)...

acl-cqc · 2026-08-05T14:50:08Z

+                let op = hugr.get_optype(n);
+                op.value_input_type(p).expect("must be dataflow edge")


For get_optype(n).value_(in/out)put_type, for value_(in/out)put_type.expect, or all three? Not opposed to any of those

acl-cqc · 2026-08-05T14:51:18Z

+                let op = hugr.get_optype(n);
+                op.value_input_type(p).expect("must be dataflow edge")


Or (Node) -> impl Iterator<Item=Type>? (+direction, or *2 for in/out)

acl-cqc · 2026-08-05T14:52:08Z

-        self.node_ports(node, dir)
-            .filter_map(move |port| sig.port_type(port).map(|typ| (port, typ.clone())))
+        let op = self.get_optype(node);
+        op.value_ports(dir)


yeah, definitely feels like giving back types as well as ports would be good here

acl-cqc · 2026-08-05T14:53:03Z

+            Direction::Incoming => (&self.just_inputs, &self.rest),
+            Direction::Outgoing => (&self.just_outputs, &self.rest),
+        };
+        head.get(port.index())


consider chaining two and then .get on that?

Chaining iterators produces a linear-time .get.

I rewrote this with simpler ifs.

acl-cqc · 2026-08-05T14:56:17Z


+    fn value_port_type(&self, port: Port) -> Option<Type> {
+        match port.direction() {
+            Direction::Incoming => self.variants.get(self.tag)?.get(port.index()).cloned(),


Just None if self.tag is out-of-bounds? In fn signature we panic (via expect)

Updated, and gave it a better error message for all cases.

acl-cqc · 2026-08-05T14:58:20Z

+    #[case::tag(tag())]
+    #[case::tail_loop(tail_loop())]
+    #[case::conditional(conditional())]
+    fn value_ports_match_signature(#[case] op: OpType) {


Good test. You might want to consider call and dfg

acl-cqc

👍 Yah ok, agreed this is the best design that works 😆

acl-cqc · 2026-09-02T12:04:12Z

    }

+    fn value_port_type(&self, port: Port) -> Option<Type> {
+        match port.direction() {


You might consider

if port.direction() == Direction::Incoming { if port.index() == 0 {return Some(Type::function(.....))} port = Port::new(Direction::Incoming, port.index() - 1); // harder than it should be? } self.signature.port_type(port)

acl-cqc · 2026-09-02T12:05:29Z

+    /// Return the TypeRow of the selected variant.
+    ///
+    /// Panics if the tag is out of bounds.
+    fn variant(&self) -> &TypeRow {


Maybe call this variant_row or similar to avoid any ambiguity

aborgna-q force-pushed the ab/envelope-perf-2 branch from c8cef65 to 589cc3a Compare July 10, 2026 13:04

aborgna-q force-pushed the ab/envelope-perf-2 branch from 589cc3a to a544401 Compare July 13, 2026 10:42

aborgna-q marked this pull request as ready for review July 13, 2026 11:34

aborgna-q requested a review from a team as a code owner July 13, 2026 11:34

aborgna-q requested a review from acl-cqc July 13, 2026 11:34

acl-cqc reviewed Jul 14, 2026

View reviewed changes

Comment thread hugr-core/src/ops.rs Outdated

acl-cqc reviewed Jul 14, 2026

View reviewed changes

aborgna-q force-pushed the ab/envelope-perf-2 branch from a544401 to 110df42 Compare July 16, 2026 13:59

aborgna-q commented Jul 31, 2026

View reviewed changes

aborgna-q requested a review from acl-cqc July 31, 2026 08:51

aborgna-q commented Jul 31, 2026

View reviewed changes

acl-cqc reviewed Aug 5, 2026

View reviewed changes

aborgna-q mentioned this pull request Aug 7, 2026

[Tracking issue][Performance]: Hugr-model import/export performance #3197

Open

4 tasks

aborgna-q force-pushed the ab/envelope-perf-2 branch from 0427210 to 256f3d1 Compare September 1, 2026 16:07

aborgna-q added 6 commits September 1, 2026 17:07

perf: Avoid cloning signatures for simple port checks

d06e853

Move new methods to OpTrait instead

2a6f020

Use new methods in HugrView::value_types

2d2b13b

Replace more signature allocs

9d1b1b8

Rewrite value_port_type code

f515a47

Improve error on invalid Tag value

3bb6709

aborgna-q force-pushed the ab/envelope-perf-2 branch from 256f3d1 to 3bb6709 Compare September 1, 2026 16:08

aborgna-q requested a review from acl-cqc September 1, 2026 16:16

acl-cqc approved these changes Sep 2, 2026

View reviewed changes

aborgna-q added 2 commits September 2, 2026 13:27

Add value_types to optype too

a3cd08c

Review comment

238ba09

aborgna-q added this pull request to the merge queue Sep 2, 2026

Merged via the queue into main with commit b15dfdb Sep 2, 2026
31 checks passed

aborgna-q deleted the ab/envelope-perf-2 branch September 2, 2026 13:09

hugrbot mentioned this pull request Sep 2, 2026

chore: release #3216

Draft

		let op = hugr.get_optype(n);
		op.value_input_type(p).expect("must be dataflow edge")

Uh oh!

Conversation

aborgna-q commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will improve performance by 9.63%

Performance Changes

Uh oh!

codecov Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aborgna-q commented Jul 13, 2026

Uh oh!

codspeed-hq Bot commented Jul 13, 2026

Why this PR is faster

Root cause: eliminating whole-Signature allocations on the hot path

Where the time went

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

acl-cqc left a comment

Choose a reason for hiding this comment

Uh oh!

aborgna-q commented Jul 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acl-cqc Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aborgna-q commented Aug 3, 2026

Uh oh!

acl-cqc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acl-cqc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

aborgna-q commented Jul 10, 2026 •

edited

Loading

codspeed-hq Bot commented Jul 10, 2026 •

edited

Loading

codecov Bot commented Jul 10, 2026 •

edited

Loading

Root cause: eliminating whole-`Signature` allocations on the hot path

acl-cqc Sep 2, 2026 •

edited

Loading