Remove needless string allocations #243

Aviac commented 2025年10月06日 09:05:42 +02:00

At a lot of places we should get away with just using properly lifetimed &str instead of String.

Go through the code and fix all the cases where this happens.

At a lot of places we should get away with just using properly lifetimed `&str` instead of `String`. Go through the code and fix all the cases where this happens.

timstoop commented 2025年10月11日 15:22:28 +02:00

I had the AI analyse the codebase and it found several patterns causing unnecessary string allocations:

Main Issues

1. option_display function (85+ usages across 23 files)

   • Returns String but is often used inside format!() macros, causing double allocation
   • Uses String::from("?") instead of "?" for the None case

2. Unnecessary String::from() in closures (~10 occurrences)

   • Pattern: .unwrap_or_else(|| String::from("?"))
   • Can be simplified to .unwrap_or("?")

3. String literals in table construction

   • String::from("Title") where &str might suffice (depends on table API)

Proposed Solutions

Option A: Use Cow<'static, str>

usestd::borrow::Cow;pubfn option_display<T: ToString>(opt: &Option<T>)-> Cow<'static,str>{opt.as_ref().map(|t|Cow::Owned(t.to_string())).unwrap_or(Cow::Borrowed("?"))}

Pros:

• Only allocates when Some, returns borrowed "?" for None
• Drop-in replacement due to Deref implementation
• Works seamlessly with format!() and other string operations
• Minimal API changes

Cons:

• Still allocates for the Some case (unavoidable since we need ToString)

Option B: Display trait wrapper

pubstruct OptionDisplay<'a,T>(&'aOption<T>);impl<'a,T: ToString>DisplayforOptionDisplay<'a,T>{fn fmt(&self,f: &mutFormatter<'_>)-> fmt::Result{match&self.0{Some(v)=>write!(f,"{}",v.to_string()),None=>write!(f,"?"),}}}pubfn option_display<T>(opt: &Option<T>)-> OptionDisplay<'_,T>{OptionDisplay(opt)}

Pros:

• Zero allocations until actually formatted
• Most efficient solution

Cons:

• More complex lifetime management
• May require changes where the result is stored/passed around
• Could break existing usage patterns

Recommendation

Option A (Cow) provides immediate benefits with minimal disruption. We can then address the remaining allocations in the table construction separately if needed.

Happy to implement whichever approach you prefer, or explore other solutions if you have different ideas.

I had the AI analyse the codebase and it found several patterns causing unnecessary string allocations: ## Main Issues 1. **`option_display` function** (85+ usages across 23 files) - Returns `String` but is often used inside `format!()` macros, causing double allocation - Uses `String::from("?")` instead of `"?"` for the None case 2. **Unnecessary `String::from()` in closures** (~10 occurrences) - Pattern: `.unwrap_or_else(|| String::from("?"))` - Can be simplified to `.unwrap_or("?")` 3. **String literals in table construction** - `String::from("Title")` where `&str` might suffice (depends on table API) ## Proposed Solutions ### Option A: Use `Cow<'static, str>` ```rust use std::borrow::Cow; pub fn option_display<T: ToString>(opt: &Option<T>) -> Cow<'static, str> { opt.as_ref() .map(|t| Cow::Owned(t.to_string())) .unwrap_or(Cow::Borrowed("?")) } ``` **Pros:** - Only allocates when `Some`, returns borrowed `"?"` for `None` - Drop-in replacement due to `Deref` implementation - Works seamlessly with `format!()` and other string operations - Minimal API changes **Cons:** - Still allocates for the `Some` case (unavoidable since we need `ToString`) ### Option B: `Display` trait wrapper ```rust pub struct OptionDisplay<'a, T>(&'a Option<T>); impl<'a, T: ToString> Display for OptionDisplay<'a, T> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match &self.0 { Some(v) => write!(f, "{}", v.to_string()), None => write!(f, "?"), } } } pub fn option_display<T>(opt: &Option<T>) -> OptionDisplay<'_, T> { OptionDisplay(opt) } ``` **Pros:** - Zero allocations until actually formatted - Most efficient solution **Cons:** - More complex lifetime management - May require changes where the result is stored/passed around - Could break existing usage patterns ## Recommendation Option A (Cow) provides immediate benefits with minimal disruption. We can then address the remaining allocations in the table construction separately if needed. Happy to implement whichever approach you prefer, or explore other solutions if you have different ideas.