mirror of
https://github.com/microsoft/regorus.git
synced 2026-08-05 02:16:11 +00:00
Rvm optimizations (#620)
* perf(rvm): fix O(n²) comprehension yield by mutating in-place Instead of cloning the entire accumulator collection on every yield iteration, use take_register + Rc::make_mut to get exclusive ownership and mutate in-place. This reduces comprehension yield from O(n²) to O(n) for both run-to-completion and suspendable execution modes. - Add RegoVM::take_register() helper that swaps register with Undefined - Comprehension yield now takes the accumulator, mutates via Rc::make_mut, and writes back — avoiding deep clones when refcount == 1 Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com> * perf(rvm): use take_register for ObjectSet, ArrayPush, SetAdd These instructions were cloning the container register (bumping Rc to 2), then calling as_object_mut/as_array_mut/as_set_mut which invokes Rc::make_mut — deep-cloning the entire collection since refcount > 1. Use take_register instead so the Rc refcount stays at 1, making Rc::make_mut a no-op and allowing in-place mutation. Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com> * perf(rvm): remove unnecessary clones in rule caching - execute_call_rule_common: move final_value into cache instead of cloning, since it is not used afterwards - finalize_rule_frame_data: add comment clarifying the clone is needed because the value is both cached and returned - Remove unnecessary .clone() on result_from_rule when setting register Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com> * rvm: avoid RuleInfo clone per rule call Replace RuleInfo.clone() (which heap-allocates name, destructuring_blocks, and potentially function_info) with a cheap Arc<Program> clone (atomic refcount bump) followed by borrowing &RuleInfo from the local Arc. This eliminates per-rule-call heap allocations. Sites changed: - execute_call_rule_common: Arc clone + borrow - execute_call_rule_suspendable: Arc clone + borrow - finalize_rule_frame_data: Arc clone + borrow - handle_rule_break_event: inline Arc clone + borrow (was get_rule_info) - handle_rule_error_event: inline Arc clone + borrow (was get_rule_info) - Removed now-unused get_rule_info method Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com> * rvm: replace bincode with postcard for serialization Remove unlinked bincode dependency. Use postcard (already a dep for rvm feature) for all binary serialization/deserialization in program serialization and tests. Also adds rvm_benchmark benchmark. Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com> * perf(rvm): cache dummy Span/Expr for builtin calls Every builtin call was allocating a Source (via from_contents), a Span, and N Ref<Expr> wrappers just to satisfy the builtin function signature. These dummy values are only used for error reporting context. Cache the dummy Span and Vec<Ref<Expr>> on the RegoVM struct. The Source and Span are created once on first builtin call; dummy Expr entries grow as needed and are reused across calls via mem::take/put-back pattern. This eliminates per-builtin-call heap allocations for Source (Rc + String + Vec<lines>), Span clones, and Rc<Expr> wrappers. Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com> * perf(rvm): round 2 allocation reduction in builtins, entry points, virtual data - Cache builtin args Vec on RegoVM (mem::take/clear/put-back pattern) - Restructure builtins_cache as two-level map for clone-free lookup - Use IndexMap::get_index() in execute_entry_point_by_index - Use mutable Vec path stack in traverse_rule_tree_subobject (push/pop) - Walk data tree and rule-result paths by reference, clone only leaf - Use mem::replace in resume() instead of cloning ExecutionState Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com> * fix(rvm): address PR review feedback - Restore cached_builtin_args on all error/early-return paths in execute_builtin_call to preserve allocation reuse - Use 1-based line/col and \"<builtin>\" filename in dummy span for clearer diagnostics - Restore result register before returning errors in comprehension mode-mismatch branches (both run-to-completion and suspendable) - Avoid clone in resume() invalid-state error path by formatting debug string before moving state back --------- Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
This commit is contained in:
Anand Krishnamoorthi
committed by
GitHub
GitHub
parent
ee3dff9a3d
commit
50c0215fdb
+33
-37
@@ -255,26 +255,22 @@ impl RegoVM {
|
||||
};
|
||||
|
||||
let result_reg = comprehension_context.result_reg;
|
||||
let current_result = self.get_register(result_reg)?.clone();
|
||||
let mode = comprehension_context.mode.clone();
|
||||
// Take ownership of the result register so Rc refcount stays at 1,
|
||||
// allowing Rc::make_mut to mutate in-place instead of deep-cloning.
|
||||
let mut current_result = self.take_register(result_reg)?;
|
||||
|
||||
let updated_result = match (mode, current_result) {
|
||||
(ComprehensionMode::Set, Value::Set(set)) => {
|
||||
let mut new_set = set.as_ref().clone();
|
||||
new_set.insert(value_to_add);
|
||||
Value::Set(crate::Rc::new(new_set))
|
||||
match (&comprehension_context.mode, &mut current_result) {
|
||||
(&ComprehensionMode::Set, &mut Value::Set(ref mut set)) => {
|
||||
crate::Rc::make_mut(set).insert(value_to_add);
|
||||
}
|
||||
(ComprehensionMode::Array, Value::Array(arr)) => {
|
||||
let mut new_arr = arr.as_ref().to_vec();
|
||||
new_arr.push(value_to_add);
|
||||
Value::Array(crate::Rc::new(new_arr))
|
||||
(&ComprehensionMode::Array, &mut Value::Array(ref mut arr)) => {
|
||||
crate::Rc::make_mut(arr).push(value_to_add);
|
||||
}
|
||||
(ComprehensionMode::Object, Value::Object(obj)) => {
|
||||
(&ComprehensionMode::Object, &mut Value::Object(ref mut obj)) => {
|
||||
if let Some(key) = key_value {
|
||||
let mut new_obj = obj.as_ref().clone();
|
||||
new_obj.insert(key, value_to_add);
|
||||
Value::Object(crate::Rc::new(new_obj))
|
||||
crate::Rc::make_mut(obj).insert(key, value_to_add);
|
||||
} else {
|
||||
self.set_register(result_reg, current_result)?;
|
||||
self.comprehension_stack.push(comprehension_context);
|
||||
return Err(VmError::InvalidIteration {
|
||||
value: Value::String(Arc::from("Object comprehension requires key")),
|
||||
@@ -283,15 +279,17 @@ impl RegoVM {
|
||||
}
|
||||
}
|
||||
(_mode, other) => {
|
||||
let offending = core::mem::replace(other, Value::Undefined);
|
||||
self.set_register(result_reg, current_result)?;
|
||||
self.comprehension_stack.push(comprehension_context);
|
||||
return Err(VmError::InvalidIteration {
|
||||
value: other,
|
||||
value: offending,
|
||||
pc: self.pc,
|
||||
});
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
self.set_register(result_reg, updated_result)?;
|
||||
self.set_register(result_reg, current_result)?;
|
||||
|
||||
if let Some(iter_state) = comprehension_context.iteration_state.as_mut() {
|
||||
match *iter_state {
|
||||
@@ -357,7 +355,6 @@ impl RegoVM {
|
||||
let (
|
||||
value_to_add,
|
||||
key_value,
|
||||
current_result,
|
||||
mode,
|
||||
result_reg_idx,
|
||||
key_reg_idx,
|
||||
@@ -384,7 +381,6 @@ impl RegoVM {
|
||||
};
|
||||
|
||||
let result_reg_idx = context.result_reg;
|
||||
let current_result = self.get_register(result_reg_idx)?.clone();
|
||||
let mode = context.mode.clone();
|
||||
let iteration_key = self.get_register(context.key_reg)?.clone();
|
||||
let iteration_value = self.get_register(context.value_reg)?.clone();
|
||||
@@ -392,7 +388,6 @@ impl RegoVM {
|
||||
(
|
||||
value_to_add,
|
||||
key_value,
|
||||
current_result,
|
||||
mode,
|
||||
result_reg_idx,
|
||||
context.key_reg,
|
||||
@@ -408,23 +403,22 @@ impl RegoVM {
|
||||
}
|
||||
};
|
||||
|
||||
let updated_result = match (mode, current_result) {
|
||||
(ComprehensionMode::Set, Value::Set(set)) => {
|
||||
let mut new_set = set.as_ref().clone();
|
||||
new_set.insert(value_to_add);
|
||||
Value::Set(crate::Rc::new(new_set))
|
||||
// Take ownership of the result register so Rc refcount stays at 1,
|
||||
// allowing Rc::make_mut to mutate in-place instead of deep-cloning.
|
||||
let mut current_result = self.take_register(result_reg_idx)?;
|
||||
|
||||
match (&mode, &mut current_result) {
|
||||
(&ComprehensionMode::Set, &mut Value::Set(ref mut set)) => {
|
||||
crate::Rc::make_mut(set).insert(value_to_add);
|
||||
}
|
||||
(ComprehensionMode::Array, Value::Array(arr)) => {
|
||||
let mut new_arr = arr.as_ref().to_vec();
|
||||
new_arr.push(value_to_add);
|
||||
Value::Array(crate::Rc::new(new_arr))
|
||||
(&ComprehensionMode::Array, &mut Value::Array(ref mut arr)) => {
|
||||
crate::Rc::make_mut(arr).push(value_to_add);
|
||||
}
|
||||
(ComprehensionMode::Object, Value::Object(obj)) => {
|
||||
(&ComprehensionMode::Object, &mut Value::Object(ref mut obj)) => {
|
||||
if let Some(key) = key_value {
|
||||
let mut new_obj = obj.as_ref().clone();
|
||||
new_obj.insert(key, value_to_add);
|
||||
Value::Object(crate::Rc::new(new_obj))
|
||||
crate::Rc::make_mut(obj).insert(key, value_to_add);
|
||||
} else {
|
||||
self.set_register(result_reg_idx, current_result)?;
|
||||
return Err(VmError::InvalidIteration {
|
||||
value: Value::String(Arc::from("Object comprehension requires key")),
|
||||
pc: self.pc,
|
||||
@@ -432,12 +426,14 @@ impl RegoVM {
|
||||
}
|
||||
}
|
||||
(_mode, other) => {
|
||||
let offending = core::mem::replace(other, Value::Undefined);
|
||||
self.set_register(result_reg_idx, current_result)?;
|
||||
return Err(VmError::InvalidIteration {
|
||||
value: other,
|
||||
value: offending,
|
||||
pc: self.pc,
|
||||
});
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
let (iteration_state_snapshot, body_start, comprehension_end) = {
|
||||
let frame = self.execution_stack.get_mut(comprehension_index).ok_or(
|
||||
@@ -489,7 +485,7 @@ impl RegoVM {
|
||||
}
|
||||
};
|
||||
|
||||
self.set_register(result_reg_idx, updated_result)?;
|
||||
self.set_register(result_reg_idx, current_result)?;
|
||||
|
||||
if let Some(state) = iteration_state_snapshot.as_ref() {
|
||||
let has_next = self.setup_next_iteration(state, key_reg_idx, value_reg_idx)?;
|
||||
|
||||
Reference in New Issue
Block a user