Skip to content

Commit 1601ea0

Browse files
⚡ Bolt: Optimize Context variable/function lookups
* Removed redundant `.clone()` operations on inner data inside `get_func` and `get_variable` when mapping `ContextValue`. * Refactored `value` to perform a single `.get(name)` lookup rather than doing `is_none()` followed by `unwrap()`. * Avoided holding `MutexGuard` across `func(Vec::new())` execution, thus dropping the lock early and preventing potential deadlock. * Documented the learning in `.jules/bolt.md`. Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
1 parent 5576973 commit 1601ea0

2 files changed

Lines changed: 10 additions & 12 deletions

File tree

.jules/bolt.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
## 2024-05-18 - Avoid double lookups and redundant clones in Context
2+
**Learning:** The `Context` manager in `src/context.rs` performed redundant hash map lookups (e.g., calling `.get(name).is_none()` followed by `.get(name).unwrap()`) and cloned values that were already owned after being returned from `get()`. By relying on a single `get()` call and moving the inner `Arc` or `Value` directly via pattern matching, we can eliminate these unnecessary double hash map accesses and redundant allocations.
3+
**Action:** Always prefer matching on `Option` returned from a single map lookup instead of performing `is_none()` checks before `unwrap()`. Furthermore, when matching on an owned value (like an `enum` variant), directly move the inner data instead of cloning it unnecessarily.

src/context.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,34 +33,29 @@ impl Context {
3333
pub fn get_func(&self, name: &str) -> Option<Arc<InnerFunction>> {
3434
let value = self.get(name)?;
3535
match value {
36-
ContextValue::Function(func) => Some(func.clone()),
36+
ContextValue::Function(func) => Some(func),
3737
ContextValue::Variable(_) => None,
3838
}
3939
}
4040

4141
pub fn get_variable(&self, name: &str) -> Option<Value> {
4242
let value = self.get(name)?;
4343
match value {
44-
ContextValue::Variable(v) => Some(v.clone()),
44+
ContextValue::Variable(v) => Some(v),
4545
ContextValue::Function(_) => None,
4646
}
4747
}
4848

4949
pub fn get(&self, name: &str) -> Option<ContextValue> {
50-
let binding = self.0.lock().unwrap();
51-
let value = binding.get(name)?;
52-
Some(value.clone())
50+
self.0.lock().unwrap().get(name).cloned()
5351
}
5452

5553
pub fn value(&self, name: &str) -> Result<Value> {
56-
let binding = self.0.lock().unwrap();
57-
if binding.get(name).is_none() {
58-
return Ok(Value::None);
59-
}
60-
let value = binding.get(name).unwrap();
54+
let value = self.get(name);
6155
match value {
62-
ContextValue::Variable(v) => Ok(v.clone()),
63-
ContextValue::Function(func) => func(Vec::new()),
56+
Some(ContextValue::Variable(v)) => Ok(v),
57+
Some(ContextValue::Function(func)) => func(Vec::new()),
58+
None => Ok(Value::None),
6459
}
6560
}
6661
}

0 commit comments

Comments
 (0)