Skip to content

Commit 75e8276

Browse files
perf(context): optimize Context map lookups to reduce lock contention and cloning
- Replaces double HashMap lookups with a single `.get().cloned()` call. - Uses pattern matching in `value()`, `get_func()`, and `get_variable()` to directly handle the cloned `ContextValue` variant without redundant `clone()` invocations on the inner types. - Crucially, allows the Mutex lock to be dropped earlier in `value()` before executing arbitrary long-running inner function closures. Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
1 parent 5576973 commit 75e8276

4 files changed

Lines changed: 15 additions & 24 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+
## 2026-03-18 - Optimize Context lookups
2+
**Learning:** In highly accessed global state structures like `Context`, redundantly cloning the encapsulated `ContextValue` struct and its underlying inner values inside lock blocks leads to measurable overhead and unnecessary allocations. Further, double hash map lookups where we check if a key exists then immediately unwrap it again, can be simply avoided using pattern matching.
3+
**Action:** Use single `.get()` calls over hash maps followed by `.cloned()`. Pattern match directly on the result to avoid redundant hash map operations. Ensure locks are dropped before executing potentially long-running functions.

benches/display_expression.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use criterion::{criterion_group, criterion_main, Criterion, black_box};
1+
use criterion::{black_box, criterion_group, criterion_main, Criterion};
22
use expression_engine::{parse_expression, ExprAST};
33

44
fn bench_display_expression(c: &mut Criterion) {

src/context.rs

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,34 +33,28 @@ 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();
61-
match value {
62-
ContextValue::Variable(v) => Ok(v.clone()),
63-
ContextValue::Function(func) => func(Vec::new()),
54+
match self.get(name) {
55+
Some(ContextValue::Variable(v)) => Ok(v),
56+
Some(ContextValue::Function(func)) => func(Vec::new()),
57+
None => Ok(Value::None),
6458
}
6559
}
6660
}

src/parser.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,22 +52,16 @@ impl<'a> fmt::Display for ExprAST<'a> {
5252
Self::Unary(op, rhs) => {
5353
write!(f, "Unary AST: Op: {}, Rhs: {}", op, rhs)
5454
}
55-
Self::Binary(op, lhs, rhs) => write!(
56-
f,
57-
"Binary AST: Op: {}, Lhs: {}, Rhs: {}",
58-
op,
59-
lhs,
60-
rhs
61-
),
55+
Self::Binary(op, lhs, rhs) => {
56+
write!(f, "Binary AST: Op: {}, Lhs: {}, Rhs: {}", op, lhs, rhs)
57+
}
6258
Self::Postfix(lhs, op) => {
6359
write!(f, "Postfix AST: Lhs: {}, Op: {}", lhs, op,)
6460
}
6561
Self::Ternary(condition, lhs, rhs) => write!(
6662
f,
6763
"Ternary AST: Condition: {}, Lhs: {}, Rhs: {}",
68-
condition,
69-
lhs,
70-
rhs
64+
condition, lhs, rhs
7165
),
7266
Self::Reference(name) => write!(f, "Reference AST: reference: {}", name),
7367
Self::Function(name, params) => {

0 commit comments

Comments
 (0)