From b129d934615162cc020f0457dd2cf257a605997c Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 27 Mar 2026 21:13:58 +0000 Subject: [PATCH] perf(value): optimize Display implementation for Value enum Optimized the `std::fmt::Display` trait implementation for the `Value` enum to reduce memory allocations and unnecessary clones. Replaced `format!()` and intermediate `String::from()` buffers with direct writes to the `std::fmt::Formatter` via `write!()`. Removed redundant `.clone()` calls during formatting to avoid unnecessary heap allocations. Trailing delimiters correctly emulate the original format output. Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com> --- .jules/bolt.md | 4 ++++ src/value.rs | 21 +++++++++------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/.jules/bolt.md b/.jules/bolt.md index 1878ef8..18b97da 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -9,3 +9,7 @@ ## 2024-05-26 - Optimize decimal conversion in Value **Learning:** `rust_decimal::Decimal` allows efficient and direct conversion to basic types like `i64` and `f64` via `to_i64` and `to_f64` using the `rust_decimal::prelude::ToPrimitive` trait. Converting to string and then parsing `val.to_string().parse()` is an anti-pattern as it incurs heap allocation overhead, which is bad for performance. When doing integer conversions from `Decimal`, it's critical to check `val.scale() == 0` first to maintain behavioral parity with string parsing (which would reject floats). **Action:** Always favor direct conversion traits like `rust_decimal::prelude::ToPrimitive` methods over stringification when converting `Decimal` values to native numeric types. Keep edge cases like `scale()` properties in mind when changing conversion methods. + +## 2024-05-27 - Avoid intermediate strings and clones in std::fmt::Display +**Learning:** For `std::fmt::Display` implementations involving collections (like Lists or Maps), building an intermediate `String` via `format!()` and `push_str()` before writing it causes unnecessary heap allocations and redundant data cloning. +**Action:** Write directly to the `std::fmt::Formatter` using `write!(f, ...)` to stream characters without allocating extra memory, and remove unnecessary `.clone()` calls during formatting. diff --git a/src/value.rs b/src/value.rs index d1b9133..025496c 100644 --- a/src/value.rs +++ b/src/value.rs @@ -17,25 +17,22 @@ pub enum Value { impl fmt::Display for Value { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::String(val) => write!(f, "value string: {}", val.clone()), - Self::Number(val) => write!(f, "value number: {}", val.clone()), - Self::Bool(val) => write!(f, "value bool: {}", val.clone()), + Self::String(val) => write!(f, "value string: {}", val), + Self::Number(val) => write!(f, "value number: {}", val), + Self::Bool(val) => write!(f, "value bool: {}", val), Self::List(values) => { - let mut s = String::from("["); + write!(f, "value list: [")?; for value in values { - s.push_str(format!("{},", value.clone()).as_str()); + write!(f, "{},", value)?; } - s.push_str("]"); - write!(f, "value list: {}", s) + write!(f, "]") } Self::Map(m) => { - let mut s = String::from("{"); + write!(f, "value map: {{")?; for (k, v) in m { - s.push_str(format!("key: {},", k.clone()).as_str()); - s.push_str(format!("value: {}; ", v.clone()).as_str()); + write!(f, "key: {},value: {}; ", k, v)?; } - s.push_str("}"); - write!(f, "value map: {}", s) + write!(f, "}}") } Self::None => write!(f, "None"), }