Skip to content

Commit a532919

Browse files
Optimize Context value lookup and lock duration
Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
1 parent 6cf9bef commit a532919

2 files changed

Lines changed: 11 additions & 12 deletions

File tree

.jules/bolt.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
11
## 2024-05-24 - Avoid vec! for constant collections in initialization loops
22
**Learning:** Initializing maps/operator managers by iterating over `vec![...]` causes unnecessary heap allocations. Using array literals `[...]` is significantly more efficient since the size is known at compile time and the arrays can be stack-allocated or embedded directly into the binary.
33
**Action:** Always prefer iterating over array literals instead of `vec![...]` for statically known collections, especially in hot paths or initialization loops.
4+
5+
## 2024-05-25 - Avoid double lookup and lock contention in Context lookup
6+
**Learning:** `HashMap::get(name)` followed by `.unwrap()` inside a Mutex lock not only does double lookup but keeps the lock longer than necessary when executing an inner function or cloning a value.
7+
**Action:** Use a single lookup, clone the value or arc into a local variable (`Option<ContextValue>`), and release the lock immediately before execution.

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)