From 92140eba26962e7f067f3c51018fae2b9c713360 Mon Sep 17 00:00:00 2001
From: Jake Lishman <jake.lishman@ibm.com>
Date: Mon, 2 Sep 2024 14:50:16 +0100
Subject: [PATCH] Avoid `ExtraInstructionAttributes` allocation on `unit="dt"`

The default value for `Instruction.unit` is `"dt"`.  Previously, the
`OperationFromPython` extraction logic would only suppress allocation of
the extra instruction attributes if all the contained fields were
`None`, but `None` is not actually a valid value of `Instruction.unit`
(which must be a string).  This meant that `OperationFromPython` would
always allocate and store extra attributes, even for the default cases.
This did not affect standard gates appended using their corresponding
`QuantumCircuit` methods (since no Python-space extraction is
performed in that case), but did affect standard calls to `append`, or
anything else that entered from Python space.

This drastically reduces the memory usage of circuits built by
`append`-like methods. Ignoring the inefficiency factor of the
heap-allocation implementation, this saves 66 bytes plus
small-allocation overhead for 2-byte heap allocations (another 14 bytes
on macOS, but will vary depending on the allocator) per standard
instruction, which is on the order of 40% memory-usage reduction.
---
 crates/circuit/src/circuit_instruction.rs | 37 ++++++++++++++++++++---
 crates/circuit/src/operations.rs          |  2 +-
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/crates/circuit/src/circuit_instruction.rs b/crates/circuit/src/circuit_instruction.rs
index b0620d78fb75..8efb4f332e5b 100644
--- a/crates/circuit/src/circuit_instruction.rs
+++ b/crates/circuit/src/circuit_instruction.rs
@@ -17,7 +17,7 @@ use numpy::IntoPyArray;
 use pyo3::basic::CompareOp;
 use pyo3::exceptions::{PyDeprecationWarning, PyTypeError};
 use pyo3::prelude::*;
-use pyo3::types::{PyList, PyTuple, PyType};
+use pyo3::types::{PyList, PyString, PyTuple, PyType};
 use pyo3::{intern, IntoPy, PyObject, PyResult};
 
 use smallvec::SmallVec;
@@ -63,6 +63,20 @@ impl ExtraInstructionAttributes {
             None
         }
     }
+
+    /// Get the Python-space version of the stored `unit`.  This evalutes the Python-space default
+    /// (`"dt"`) value if we're storing a `None`.
+    pub fn py_unit(&self, py: Python) -> Py<PyString> {
+        self.unit
+            .as_deref()
+            .map(|unit| <&str as IntoPy<Py<PyString>>>::into_py(unit, py))
+            .unwrap_or_else(|| Self::default_unit(py).clone().unbind())
+    }
+
+    /// Get the Python-space default value for the `unit` field.
+    pub fn default_unit(py: Python) -> &Bound<PyString> {
+        intern!(py, "dt")
+    }
 }
 
 /// A single instruction in a :class:`.QuantumCircuit`, comprised of the :attr:`operation` and
@@ -267,10 +281,17 @@ impl CircuitInstruction {
     }
 
     #[getter]
-    fn unit(&self) -> Option<&str> {
+    fn unit(&self, py: Python) -> Py<PyString> {
+        // Python space uses `"dt"` as the default, whereas we simply don't store the extra
+        // attributes at all if they're none.
         self.extra_attrs
             .as_ref()
-            .and_then(|attrs| attrs.unit.as_deref())
+            .map(|attrs| attrs.py_unit(py))
+            .unwrap_or_else(|| {
+                ExtraInstructionAttributes::default_unit(py)
+                    .clone()
+                    .unbind()
+            })
     }
 
     /// Is the :class:`.Operation` contained in this instruction a Qiskit standard gate?
@@ -524,10 +545,18 @@ impl<'py> FromPyObject<'py> for OperationFromPython {
                 .map(|params| params.unwrap_or_default())
         };
         let extract_extra = || -> PyResult<_> {
+            let unit = {
+                // We accept Python-space `None` or `"dt"` as both meaning the default `"dt"`.
+                let raw_unit = ob.getattr(intern!(py, "unit"))?;
+                (!(raw_unit.is_none()
+                    || raw_unit.eq(ExtraInstructionAttributes::default_unit(py))?))
+                .then(|| raw_unit.extract::<String>())
+                .transpose()?
+            };
             Ok(ExtraInstructionAttributes::new(
                 ob.getattr(intern!(py, "label"))?.extract()?,
                 ob.getattr(intern!(py, "duration"))?.extract()?,
-                ob.getattr(intern!(py, "unit"))?.extract()?,
+                unit,
                 ob.getattr(intern!(py, "condition"))?.extract()?,
             )
             .map(Box::from))
diff --git a/crates/circuit/src/operations.rs b/crates/circuit/src/operations.rs
index f6e087a5680f..8cc2256af518 100644
--- a/crates/circuit/src/operations.rs
+++ b/crates/circuit/src/operations.rs
@@ -435,7 +435,7 @@ impl StandardGate {
         if let Some(extra) = extra_attrs {
             let kwargs = [
                 ("label", extra.label.to_object(py)),
-                ("unit", extra.unit.to_object(py)),
+                ("unit", extra.py_unit(py).into_any()),
                 ("duration", extra.duration.to_object(py)),
             ]
             .into_py_dict_bound(py);