Skip to content

Commit dd86f34

Browse files
committed
perf(embedded): stop paying per value in the conversion layer
value_to_py runs for every column of every row, so anything it does per call is multiplied by the size of the result set. Three costs there were avoidable: The tag lookup went through the import machinery and an attribute fetch on every converted value, to reach a class that never changes. It is now resolved once per process. Measured at 372 ns per value: 0.11 ms on a 300-row result, 37 ms on 100k. A failed lookup is cached too, since a package that is not importable at the first conversion will not become importable later in the same process. A path hop cloned its type string although the arm owns the path and drops it on the way out. Consuming it moves the string instead of duplicating it once per hop. Vectors, a path's node list and each multi-vector row grew by append with their length already known. Sizing the list once measures 79.52 -> 78.94 ms best and 81.14 -> 79.91 ms median on 200 rows of 768 floats, which is about a percent and near the noise on that sample, but it points the same way in both statistics and replaces a loop with one line. An embedding is the one value here that routinely runs to thousands of elements. Value::Array keeps its append loop on purpose: its items need a fallible recursive conversion, so sizing it once would mean collecting into an intermediate Vec and trading a growth reallocation for a whole allocation.
1 parent b03997a commit dd86f34

1 file changed

Lines changed: 44 additions & 25 deletions

File tree

coordinode-embedded/src/lib.rs

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use coordinode_core::graph::types::{GeoValue, PathRel, PathValue, Value};
2121
use coordinode_embed::{Database, DatabaseError};
2222
use pyo3::exceptions::{PyRuntimeError, PyValueError};
2323
use pyo3::prelude::*;
24+
use pyo3::sync::GILOnceCell;
2425
use pyo3::types::{PyBytes, PyDict, PyList};
2526
use rmpv::Value as MsgpackValue;
2627

@@ -34,21 +35,43 @@ use rmpv::Value as MsgpackValue;
3435
/// property's type. That module re-exports `coordinode`'s definition when that
3536
/// package is installed and defines an equivalent when it is not, so the tag
3637
/// holds for a standalone install of the embedded engine too.
38+
/// Looked up once per process rather than per value: `value_to_py` runs for
39+
/// every column of every row, and the import machinery plus the attribute
40+
/// fetch are the same two lookups every time. A failed lookup is cached too,
41+
/// since a package that is not importable at the first conversion will not
42+
/// become importable later in the same process.
43+
static MULTI_VECTOR_TYPE: GILOnceCell<Option<Py<PyAny>>> = GILOnceCell::new();
44+
3745
fn multi_vector_type(py: Python<'_>) -> Option<Bound<'_, PyAny>> {
38-
py.import("coordinode_embedded._types")
39-
.and_then(|m| m.getattr("MultiVector"))
40-
.ok()
46+
MULTI_VECTOR_TYPE
47+
.get_or_init(py, || {
48+
py.import("coordinode_embedded._types")
49+
.and_then(|m| m.getattr("MultiVector"))
50+
.map(Bound::unbind)
51+
.ok()
52+
})
53+
.as_ref()
54+
.map(|tag| tag.bind(py).clone())
4155
}
4256

4357
/// The `Path` tag, from this package's own `_types` module.
4458
///
4559
/// A dict subclass, for the same reason as [`multi_vector_type`]: without the
4660
/// tag a path read back is an ordinary mapping, and writing it out again would
4761
/// store a map.
62+
/// Cached for the same reason as [`MULTI_VECTOR_TYPE`].
63+
static PATH_TYPE: GILOnceCell<Option<Py<PyAny>>> = GILOnceCell::new();
64+
4865
fn path_type(py: Python<'_>) -> Option<Bound<'_, PyAny>> {
49-
py.import("coordinode_embedded._types")
50-
.and_then(|m| m.getattr("Path"))
51-
.ok()
66+
PATH_TYPE
67+
.get_or_init(py, || {
68+
py.import("coordinode_embedded._types")
69+
.and_then(|m| m.getattr("Path"))
70+
.map(Bound::unbind)
71+
.ok()
72+
})
73+
.as_ref()
74+
.map(|tag| tag.bind(py).clone())
5275
}
5376

5477
fn value_to_py(py: Python<'_>, v: Value) -> PyResult<PyObject> {
@@ -62,13 +85,11 @@ fn value_to_py(py: Python<'_>, v: Value) -> PyResult<PyObject> {
6285
Value::String(s) => Ok(s.into_pyobject(py)?.into_any().unbind()),
6386
// Timestamp: expose as raw microseconds; callers use datetime.fromtimestamp(ts/1e6)
6487
Value::Timestamp(ts) => Ok(ts.into_pyobject(py)?.into_any().unbind()),
65-
Value::Vector(v) => {
66-
let list = PyList::empty(py);
67-
for x in v {
68-
list.append(x)?;
69-
}
70-
Ok(list.into_any().unbind())
71-
}
88+
// Sized from the vector's length in one go. Growing by append costs a
89+
// reallocation every time the list outgrows its capacity, and an
90+
// embedding is the one value here that routinely runs to hundreds or
91+
// thousands of elements.
92+
Value::Vector(v) => Ok(PyList::new(py, v)?.into_any().unbind()),
7293
Value::Array(arr) => {
7394
let list = PyList::empty(py);
7495
for item in arr {
@@ -103,11 +124,8 @@ fn value_to_py(py: Python<'_>, v: Value) -> PyResult<PyObject> {
103124
Value::MultiVector(rows) => {
104125
let outer = PyList::empty(py);
105126
for row in rows {
106-
let inner = PyList::empty(py);
107-
for x in row {
108-
inner.append(x)?;
109-
}
110-
outer.append(inner)?;
127+
// Each row's width is known, so the list is sized once.
128+
outer.append(PyList::new(py, row)?)?;
111129
}
112130
match multi_vector_type(py) {
113131
Some(cls) => Ok(cls.call1((outer,))?.unbind()),
@@ -118,16 +136,17 @@ fn value_to_py(py: Python<'_>, v: Value) -> PyResult<PyObject> {
118136
// it runs through and the relationship hops between them.
119137
Value::Path(path) => {
120138
let d = PyDict::new(py);
121-
let nodes = PyList::empty(py);
122-
for n in &path.nodes {
123-
nodes.append(*n)?;
124-
}
125-
d.set_item("nodes", nodes)?;
139+
// Sized from the length that is already known, rather than grown
140+
// by repeated append.
141+
d.set_item("nodes", PyList::new(py, path.nodes)?)?;
126142

127143
let rels = PyList::empty(py);
128-
for rel in &path.rels {
144+
for rel in path.rels {
129145
let r = PyDict::new(py);
130-
r.set_item("type", rel.edge_type.clone())?;
146+
// The path is owned here and dropped on the way out, so the
147+
// type moves into the Python string instead of being copied
148+
// once per hop for it.
149+
r.set_item("type", rel.edge_type)?;
131150
r.set_item("source", rel.source)?;
132151
r.set_item("target", rel.target)?;
133152
rels.append(r)?;

0 commit comments

Comments
 (0)