diff --git a/.Jules/bolt.md b/.Jules/bolt.md index 851f729..7204a84 100644 --- a/.Jules/bolt.md +++ b/.Jules/bolt.md @@ -4,3 +4,6 @@ ## 2024-08-16 - String Formatting Optimization **Learning:** `push_str(&format!(...))` allocates a temporary `String` on the heap before appending to the target `String`. **Action:** Use `write!(target_string, ...)` or `writeln!(target_string, ...)` from `std::fmt::Write` to append formatted data directly to the target buffer, avoiding the intermediate allocation. Note that `writeln!` is preferred by clippy (`clippy::write-with-newline`) over `write!` with a trailing `\n`. +## 2024-05-18 - Optimized format_security_list +**Learning:** `format_security_list` was returning a new string, allocating an intermediate `Vec`, and calling `format!` in a loop, causing unnecessary heap allocations during config serialization, especially for large vetted/compromised lists. +**Action:** Changed the function to take a `&mut String` buffer, replaced `Vec::push` and `format!` with `std::fmt::Write::write!`, and updated `to_formatted_string` to pass the target buffer directly. This eliminates `O(N)` string and vector allocations. diff --git a/Cargo.lock b/Cargo.lock index 46f6e62..91d06df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -301,9 +301,9 @@ checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" [[package]] name = "chacha20" -version = "0.10.1" +version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d524456ba66e72eb8b115ff89e01e497f8e6d11d78b70b1aa13c0fbd97540a81" +checksum = "65c35e4b699c7e15ccbe7ee35c005e4fc0a278d22238a2857e6ce2dadeda1b06" dependencies = [ "cfg-if", "cpufeatures 0.3.0", diff --git a/src/config.rs b/src/config.rs index 99cc026..9eed8cb 100644 --- a/src/config.rs +++ b/src/config.rs @@ -451,7 +451,7 @@ impl Config { toml_str.push('\n'); } toml_str.push_str("vetted = "); - toml_str.push_str(&format_security_list(vetted)); + format_security_list(&mut toml_str, vetted); toml_str.push('\n'); } @@ -460,7 +460,7 @@ impl Config { toml_str.push('\n'); } toml_str.push_str("compromised = "); - toml_str.push_str(&format_security_list(compromised)); + format_security_list(&mut toml_str, compromised); toml_str.push('\n'); } @@ -468,22 +468,23 @@ impl Config { } } -fn format_security_list(list: &[SecurityEntry]) -> String { +fn format_security_list(s: &mut String, list: &[SecurityEntry]) { if list.is_empty() { - return "[]".to_string(); + s.push_str("[]"); + return; } - let mut s = "[\n".to_string(); + use std::fmt::Write; + s.push_str("[\n"); for (i, entry) in list.iter().enumerate() { - let mut parts = Vec::new(); - parts.push(format!("ref = \"{}\"", entry.reference)); + let _ = write!(s, " {{ ref = \"{}\"", entry.reference); if let Some(ref tag) = entry.tag { - parts.push(format!("tag = \"{}\"", tag)); + let _ = write!(s, ", tag = \"{}\"", tag); } if let Some(ref ts) = entry.timestamp { - parts.push(format!("timestamp = \"{}\"", ts)); + let _ = write!(s, ", timestamp = \"{}\"", ts); } - use std::fmt::Write; - let _ = write!(s, " {{ {} }}", parts.join(", ")); + s.push_str(" }"); + if i < list.len() - 1 { s.push_str(",\n"); } else { @@ -491,7 +492,6 @@ fn format_security_list(list: &[SecurityEntry]) -> String { } } s.push(']'); - s } /// Represents an entry in the vetted or compromised lists, which can include a version and timestamp. @@ -989,26 +989,29 @@ mod tests { fn test_format_security_list_fn() { use super::{format_security_list, SecurityEntry}; + let mut buf = String::new(); let empty: Vec = vec![]; - assert_eq!(format_security_list(&empty), "[]"); + format_security_list(&mut buf, &empty); + assert_eq!(buf, "[]"); let single_ref = vec![SecurityEntry { reference: "actions/checkout@sha".to_string(), tag: None, timestamp: None, }]; - assert_eq!( - format_security_list(&single_ref), - "[\n { ref = \"actions/checkout@sha\" }\n]" - ); + buf.clear(); + format_security_list(&mut buf, &single_ref); + assert_eq!(buf, "[\n { ref = \"actions/checkout@sha\" }\n]"); let single_tag = vec![SecurityEntry { reference: "actions/checkout@sha".to_string(), tag: Some("v4".to_string()), timestamp: None, }]; + buf.clear(); + format_security_list(&mut buf, &single_tag); assert_eq!( - format_security_list(&single_tag), + buf, "[\n { ref = \"actions/checkout@sha\", tag = \"v4\" }\n]" ); @@ -1017,8 +1020,10 @@ mod tests { tag: Some("v4".to_string()), timestamp: Some("2024-01-01T00:00:00Z".to_string()), }]; + buf.clear(); + format_security_list(&mut buf, &single_full); assert_eq!( - format_security_list(&single_full), + buf, "[\n { ref = \"actions/checkout@sha\", tag = \"v4\", timestamp = \"2024-01-01T00:00:00Z\" }\n]" ); @@ -1034,8 +1039,10 @@ mod tests { timestamp: Some("2024-01-02T00:00:00Z".to_string()), }, ]; + buf.clear(); + format_security_list(&mut buf, &multiple); assert_eq!( - format_security_list(&multiple), + buf, "[\n { ref = \"actions/checkout@sha1\", tag = \"v3\" },\n { ref = \"actions/setup-node@sha2\", timestamp = \"2024-01-02T00:00:00Z\" }\n]" ); }