Skip to content

Commit 3f123c1

Browse files
perf: Optimize Value Display to use Formatter directly
Optimized `fmt::Display` for `Value` enum to avoid intermediate string allocations (`String::from`, `format!()`) and redundant `.clone()` calls. The implementation now writes directly to `std::fmt::Formatter`. Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
1 parent 9a4a6cc commit 3f123c1

2 files changed

Lines changed: 41 additions & 12 deletions

File tree

benches/display_value.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
use criterion::{black_box, criterion_group, criterion_main, Criterion};
2+
use expression_engine::Value;
3+
4+
fn bench_display_value(c: &mut Criterion) {
5+
let mut list = Vec::new();
6+
for i in 0..10 {
7+
list.push(Value::from(i));
8+
}
9+
10+
let mut map = Vec::new();
11+
for i in 0..10 {
12+
map.push((Value::from(format!("key{}", i)), Value::from(i)));
13+
}
14+
15+
let val = Value::List(vec![
16+
Value::from("hello"),
17+
Value::from(42),
18+
Value::from(true),
19+
Value::List(list),
20+
Value::Map(map),
21+
]);
22+
23+
c.bench_function("display_value", |b| {
24+
b.iter(|| format!("{}", black_box(&val)))
25+
});
26+
}
27+
28+
criterion_group!(benches, bench_display_value);
29+
criterion_main!(benches);

src/value.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,29 @@ pub enum Value {
1313
None,
1414
}
1515

16+
// Optimization: Write directly to `std::fmt::Formatter` instead of creating intermediate
17+
// strings and calling `format!()` in a loop.
18+
// Impact: Reduces heap allocations, avoiding O(N) allocations for Lists and Maps.
1619
#[cfg(not(tarpaulin_include))]
1720
impl fmt::Display for Value {
1821
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
1922
match self {
20-
Self::String(val) => write!(f, "value string: {}", val.clone()),
21-
Self::Number(val) => write!(f, "value number: {}", val.clone()),
22-
Self::Bool(val) => write!(f, "value bool: {}", val.clone()),
23+
Self::String(val) => write!(f, "value string: {}", val),
24+
Self::Number(val) => write!(f, "value number: {}", val),
25+
Self::Bool(val) => write!(f, "value bool: {}", val),
2326
Self::List(values) => {
24-
let mut s = String::from("[");
27+
write!(f, "value list: [")?;
2528
for value in values {
26-
s.push_str(format!("{},", value.clone()).as_str());
29+
write!(f, "{},", value)?;
2730
}
28-
s.push_str("]");
29-
write!(f, "value list: {}", s)
31+
write!(f, "]")
3032
}
3133
Self::Map(m) => {
32-
let mut s = String::from("{");
34+
write!(f, "value map: {{")?;
3335
for (k, v) in m {
34-
s.push_str(format!("key: {},", k.clone()).as_str());
35-
s.push_str(format!("value: {}; ", v.clone()).as_str());
36+
write!(f, "key: {},value: {}; ", k, v)?;
3637
}
37-
s.push_str("}");
38-
write!(f, "value map: {}", s)
38+
write!(f, "}}")
3939
}
4040
Self::None => write!(f, "None"),
4141
}

0 commit comments

Comments
 (0)