Skip to content

Commit b8d1c05

Browse files
timfennisclaude
andauthored
perf(vm): borrow callee name in vec dispatch instead of allocating 🪢 (#155)
## Summary `dispatch_vec_call` and `dispatch_vec_call_dynamic` both eagerly built an `Option<String>` for the callee's name on every call, then only read it from rarely-taken error branches (the overload-not-found `Err` and the `call_callback` `map_err` closure). The success path threw the `String` away. Same shape of bug as the GetIterator fix in #147. ## Changes - `dispatch_vec_call`: borrow `&str` directly from `scalars.first().and_then(|f| f.name())`. The slice is a caller-owned parameter, so the borrow lifetime is independent of `&mut self` and the `map_err` closure can capture it freely. - `dispatch_vec_call_dynamic`: resolve the first vec candidate once into a held `Rc<Object>`, then borrow `&str` out of it. The `resolve_var` call already happened inside the old `callee_name()`; the `.to_string()` is what's gone. - `Vm::callee_name()` itself is kept — it's still used from the regular `Call` opcode's "no function found" error path, where the allocation is fine because we're already on an error path. ## Caveat — perf impact is barely measurable `vec_hot_loop` (200k–2M `(int,int) + (int,int)` calls): | Iters | Baseline | This PR | |---|---|---| | 200k | 39.0 ± 3.4 ms | 38.6 ± 3.1 ms | | 2M | 336.2 ± 3.8 ms | 334.5 ± 4.1 ms | ≈1.01× — within noise. `perf` confirms ~13% of total time goes to malloc/free, but the eliminated allocation is one small `String` (operator name like `"+"`) per outer vec call, dwarfed by `Function::clone`, the per-call `Vec` allocations for `arg_values`/`elem_args`/`results`, and the final `Rc::new(Object::Tuple(...))`. Unlike the GetIterator case, there's no deep recursive walk being saved here. So this is more of a code-cleanliness/correctness fix (no wasted allocation on the hot path; `&str` reads more naturally than `Option<String>`) than a real perf win. Happy to drop it if you'd rather not carry the churn. 🤖 PR description generated by Claude. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
1 parent 7c2a345 commit b8d1c05

1 file changed

Lines changed: 24 additions & 7 deletions

File tree

‎ndc_vm/src/vm.rs‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -893,13 +893,27 @@ impl Vm {
893893
debug_assert!(!vec_candidates.is_empty());
894894

895895
let arg_start = self.stack.len() - args;
896-
let callee_name = self.callee_name(args);
897-
898896
let arg_values: Vec<Value> = self.stack.split_off(arg_start);
899897
self.stack.pop(); // discard the callee slot
900898

901899
let frame_pointer = self.frames.last().expect("no frame").frame_pointer;
902900

901+
// Resolve one candidate up front so the error paths can borrow its name
902+
// as `&str` instead of allocating a `String` on every successful call.
903+
let callee_fn: Option<Rc<Object>> =
904+
vec_candidates
905+
.first()
906+
.and_then(|var| match self.resolve_var(var, frame_pointer) {
907+
Value::Object(obj) if matches!(obj.as_ref(), Object::Function(_)) => Some(obj),
908+
_ => None,
909+
});
910+
let callee_name: Option<&str> = callee_fn.as_ref().and_then(|obj| {
911+
let Object::Function(f) = obj.as_ref() else {
912+
return None;
913+
};
914+
f.name()
915+
});
916+
903917
let mut elem_args: Vec<Value> = Vec::with_capacity(args);
904918
let mut results: Vec<Value> = Vec::with_capacity(axis_len);
905919
// Cached last-match Function. Homogeneous tuples reuse this across
@@ -935,7 +949,7 @@ impl Vm {
935949
.map(|v| v.static_type().to_string())
936950
.collect::<Vec<_>>()
937951
.join(", ");
938-
let name = callee_name.as_deref().unwrap_or("?");
952+
let name = callee_name.unwrap_or("?");
939953
return Err(VmError::new(
940954
format!("no overload of '{name}' accepts element {i}: ({element_types})"),
941955
span,
@@ -946,7 +960,7 @@ impl Vm {
946960
};
947961

948962
let result = self.call_callback(scalar, &elem_args).map_err(|mut e| {
949-
let prefix = match &callee_name {
963+
let prefix = match callee_name {
950964
Some(name) => format!("while vectorising '{name}' at index {i}: "),
951965
None => format!("while vectorising at index {i}: "),
952966
};
@@ -973,7 +987,10 @@ impl Vm {
973987
span: Span,
974988
) -> Result<(), VmError> {
975989
let arg_start = self.stack.len() - args;
976-
let callee_name = self.callee_name(args);
990+
// Borrow the callee name straight off the resolved scalars — its lifetime
991+
// is tied to the caller-owned slice, not to `self`, so the error paths
992+
// can use it without allocating a `String` on the success path.
993+
let callee_name: Option<&str> = scalars.first().and_then(|f| f.name());
977994

978995
// Materialise the broadcast arguments up front so the inner
979996
// call_callback can hold &mut self without conflicting with stack
@@ -1017,7 +1034,7 @@ impl Vm {
10171034
.map(|v| v.static_type().to_string())
10181035
.collect::<Vec<_>>()
10191036
.join(", ");
1020-
let name = callee_name.as_deref().unwrap_or("?");
1037+
let name = callee_name.unwrap_or("?");
10211038
return Err(VmError::new(
10221039
format!("no overload of '{name}' accepts element {i}: ({element_types})"),
10231040
span,
@@ -1028,7 +1045,7 @@ impl Vm {
10281045
};
10291046

10301047
let result = self.call_callback(scalar, &elem_args).map_err(|mut e| {
1031-
let prefix = match &callee_name {
1048+
let prefix = match callee_name {
10321049
Some(name) => format!("while vectorising '{name}' at index {i}: "),
10331050
None => format!("while vectorising at index {i}: "),
10341051
};

0 commit comments

Comments
 (0)