Reduce memory allocations for SDP marshal (#596)

* Performance: reduce string allocations

* Performance: cosmetic improvement

Author: Hartigan

sdp/src/description/common.rs

@@ -33,14 +33,14 @@ pub struct Address {
 
 impl fmt::Display for Address {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let mut parts = vec![self.address.to_owned()];
+        write!(f, "{}", self.address)?;
         if let Some(t) = &self.ttl {
-            parts.push(t.to_string());
+            write!(f, "/{}", t)?;
         }
         if let Some(r) = &self.range {
-            parts.push(r.to_string());
+            write!(f, "/{}", r)?;
         }
-        write!(f, "{}", parts.join("/"))
+        Ok(())
     }
 }
 

sdp/src/description/media.rs

@@ -142,10 +142,11 @@ impl MediaDescription {
         fmtp: String,
     ) -> Self {
         self.media_name.formats.push(payload_type.to_string());
-        let mut rtpmap = format!("{payload_type} {name}/{clockrate}");
-        if channels > 0 {
-            rtpmap += format!("/{channels}").as_str();
-        }
+        let rtpmap = if channels > 0 {
+            format!("{payload_type} {name}/{clockrate}/{channels}")
+        } else {
+            format!("{payload_type} {name}/{clockrate}")
+        };
 
         if !fmtp.is_empty() {
             self.with_value_attribute("rtpmap".to_string(), rtpmap)
@@ -236,13 +237,23 @@ pub struct MediaName {
 
 impl fmt::Display for MediaName {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let s = [
-            self.media.clone(),
-            self.port.to_string(),
-            self.protos.join("/"),
-            self.formats.join(" "),
-        ];
-        write!(f, "{}", s.join(" "))
+        write!(f, "{} {}", self.media, self.port)?;
+
+        let mut first = true;
+        for part in &self.protos {
+            if first {
+                first = false;
+                write!(f, " {}", part)?;
+            } else {
+                write!(f, "/{}", part)?;
+            }
+        }
+
+        for part in &self.formats {
+            write!(f, " {}", part)?;
+        }
+
+        Ok(())
     }
 }
 

sdp/src/description/session.rs

@@ -150,11 +150,12 @@ pub struct RepeatTime {
 
 impl fmt::Display for RepeatTime {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let mut fields = vec![format!("{}", self.interval), format!("{}", self.duration)];
+        write!(f, "{} {}", self.interval, self.duration)?;
+
         for value in &self.offsets {
-            fields.push(format!("{value}"));
+            write!(f, " {value}")?;
         }
-        write!(f, "{}", fields.join(" "))
+        Ok(())
     }
 }
 
@@ -234,6 +235,59 @@ pub struct SessionDescription {
     pub media_descriptions: Vec<MediaDescription>,
 }
 
+impl fmt::Display for SessionDescription {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write_key_value(f, "v=", Some(&self.version))?;
+        write_key_value(f, "o=", Some(&self.origin))?;
+        write_key_value(f, "s=", Some(&self.session_name))?;
+
+        write_key_value(f, "i=", self.session_information.as_ref())?;
+
+        if let Some(uri) = &self.uri {
+            write_key_value(f, "u=", Some(uri))?;
+        }
+        write_key_value(f, "e=", self.email_address.as_ref())?;
+        write_key_value(f, "p=", self.phone_number.as_ref())?;
+        if let Some(connection_information) = &self.connection_information {
+            write_key_value(f, "c=", Some(&connection_information))?;
+        }
+
+        for bandwidth in &self.bandwidth {
+            write_key_value(f, "b=", Some(&bandwidth))?;
+        }
+        for time_description in &self.time_descriptions {
+            write_key_value(f, "t=", Some(&time_description.timing))?;
+            for repeat_time in &time_description.repeat_times {
+                write_key_value(f, "r=", Some(&repeat_time))?;
+            }
+        }
+
+        write_key_slice_of_values(f, "z=", &self.time_zones)?;
+
+        write_key_value(f, "k=", self.encryption_key.as_ref())?;
+        for attribute in &self.attributes {
+            write_key_value(f, "a=", Some(&attribute))?;
+        }
+
+        for media_description in &self.media_descriptions {
+            write_key_value(f, "m=", Some(&media_description.media_name))?;
+            write_key_value(f, "i=", media_description.media_title.as_ref())?;
+            if let Some(connection_information) = &media_description.connection_information {
+                write_key_value(f, "c=", Some(&connection_information))?;
+            }
+            for bandwidth in &media_description.bandwidth {
+                write_key_value(f, "b=", Some(&bandwidth))?;
+            }
+            write_key_value(f, "k=", media_description.encryption_key.as_ref())?;
+            for attribute in &media_description.attributes {
+                write_key_value(f, "a=", Some(&attribute))?;
+            }
+        }
+
+        Ok(())
+    }
+}
+
 /// Reset cleans the SessionDescription, and sets all fields back to their default values
 impl SessionDescription {
     /// API to match draft-ietf-rtcweb-jsep
@@ -399,61 +453,7 @@ impl SessionDescription {
     ///    k=* (encryption key)
     ///    a=* (zero or more media attribute lines)
     pub fn marshal(&self) -> String {
-        let mut result = String::new();
-
-        result += key_value_build("v=", Some(&self.version.to_string())).as_str();
-        result += key_value_build("o=", Some(&self.origin.to_string())).as_str();
-        result += key_value_build("s=", Some(&self.session_name)).as_str();
-
-        result += key_value_build("i=", self.session_information.as_ref()).as_str();
-
-        if let Some(uri) = &self.uri {
-            result += key_value_build("u=", Some(&format!("{uri}"))).as_str();
-        }
-        result += key_value_build("e=", self.email_address.as_ref()).as_str();
-        result += key_value_build("p=", self.phone_number.as_ref()).as_str();
-        if let Some(connection_information) = &self.connection_information {
-            result += key_value_build("c=", Some(&connection_information.to_string())).as_str();
-        }
-
-        for bandwidth in &self.bandwidth {
-            result += key_value_build("b=", Some(&bandwidth.to_string())).as_str();
-        }
-        for time_description in &self.time_descriptions {
-            result += key_value_build("t=", Some(&time_description.timing.to_string())).as_str();
-            for repeat_time in &time_description.repeat_times {
-                result += key_value_build("r=", Some(&repeat_time.to_string())).as_str();
-            }
-        }
-        if !self.time_zones.is_empty() {
-            let mut time_zones = vec![];
-            for time_zone in &self.time_zones {
-                time_zones.push(time_zone.to_string());
-            }
-            result += key_value_build("z=", Some(&time_zones.join(" "))).as_str();
-        }
-        result += key_value_build("k=", self.encryption_key.as_ref()).as_str();
-        for attribute in &self.attributes {
-            result += key_value_build("a=", Some(&attribute.to_string())).as_str();
-        }
-
-        for media_description in &self.media_descriptions {
-            result +=
-                key_value_build("m=", Some(&media_description.media_name.to_string())).as_str();
-            result += key_value_build("i=", media_description.media_title.as_ref()).as_str();
-            if let Some(connection_information) = &media_description.connection_information {
-                result += key_value_build("c=", Some(&connection_information.to_string())).as_str();
-            }
-            for bandwidth in &media_description.bandwidth {
-                result += key_value_build("b=", Some(&bandwidth.to_string())).as_str();
-            }
-            result += key_value_build("k=", media_description.encryption_key.as_ref()).as_str();
-            for attribute in &media_description.attributes {
-                result += key_value_build("a=", Some(&attribute.to_string())).as_str();
-            }
-        }
-
-        result
+        self.to_string()
     }
 
     /// Unmarshal is the primary function that deserializes the session description

sdp/src/extmap/mod.rs

@@ -37,20 +37,21 @@ pub struct ExtMap {
 
 impl fmt::Display for ExtMap {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let mut output = format!("{}", self.value);
+        write!(f, "{}", self.value)?;
+
         if self.direction != Direction::Unspecified {
-            output += format!("/{}", self.direction).as_str();
+            write!(f, "/{}", self.direction)?;
         }
 
         if let Some(uri) = &self.uri {
-            output += format!(" {uri}").as_str();
+            write!(f, " {uri}")?;
         }
 
         if let Some(ext_attr) = &self.ext_attr {
-            output += format!(" {ext_attr}").as_str();
+            write!(f, " {ext_attr}")?;
         }
 
-        write!(f, "{output}")
+        Ok(())
     }
 }
 

sdp/src/lexer/mod.rs

@@ -1,3 +1,5 @@
+use core::fmt;
+use std::fmt::Display;
 use std::io;
 use std::io::SeekFrom;
 
@@ -57,10 +59,39 @@ pub fn index_of(element: &str, data: &[&str]) -> i32 {
     -1
 }
 
-pub fn key_value_build(key: &str, value: Option<&String>) -> String {
-    if let Some(val) = value {
-        format!("{key}{val}{END_LINE}")
-    } else {
-        "".to_string()
+pub fn write_key_value<W: fmt::Write, V: Display>(
+    writer: &mut W,
+    key: &str,
+    value: Option<V>,
+) -> fmt::Result {
+    let Some(value) = value else {
+        return Ok(());
+    };
+
+    write!(writer, "{key}{value}{END_LINE}")
+}
+
+pub fn write_key_slice_of_values<W: fmt::Write, V: Display>(
+    writer: &mut W,
+    key: &str,
+    value: &[V],
+) -> fmt::Result {
+    if value.is_empty() {
+        return Ok(());
+    }
+
+    let mut first = true;
+
+    write!(writer, "{key}")?;
+    for val in value {
+        if first {
+            first = false;
+            write!(writer, "{val}")?;
+        } else {
+            write!(writer, " {val}")?;
+        }
     }
+    write!(writer, "{END_LINE}")?;
+
+    Ok(())
 }

sdp/src/util/mod.rs

@@ -93,14 +93,21 @@ impl fmt::Display for Codec {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
             f,
-            "{} {}/{}/{} ({}) [{}]",
-            self.payload_type,
-            self.name,
-            self.clock_rate,
-            self.encoding_parameters,
-            self.fmtp,
-            self.rtcp_feedback.join(", "),
-        )
+            "{} {}/{}/{} ({}) [",
+            self.payload_type, self.name, self.clock_rate, self.encoding_parameters, self.fmtp,
+        )?;
+
+        let mut first = true;
+        for part in &self.rtcp_feedback {
+            if first {
+                first = false;
+                write!(f, "{part}")?;
+            } else {
+                write!(f, ", {part}")?;
+            }
+        }
+
+        write!(f, "]")
     }
 }