ref(schema): Avoid intermediate string allocations (#4024)

Removes redundant string allocations when serializing types that have different in-memory representations. Micro-benchmarks of the `Annotated::to_json` method show speedups between 10% and 18% for medium to large payloads.
getsentry · Sep 11, 2024 · 8c3c3c2 · 8c3c3c2
1 parent 25a3b7a
commit 8c3c3c2
Show file tree

Hide file tree

Showing 12 changed files with 52 additions and 34 deletions.
diff --git a/relay-base-schema/src/events.rs b/relay-base-schema/src/events.rs
@@ -132,14 +132,14 @@ impl IntoValue for EventType {
     where
         Self: Sized,
     {
-        Value::String(format!("{self}"))
+        Value::String(self.to_string())
     }
 
     fn serialize_payload<S>(&self, s: S, _behavior: SkipSerialization) -> Result<S::Ok, S::Error>
     where
         Self: Sized,
         S: serde::Serializer,
     {
-        Serialize::serialize(&self.to_string(), s)
+        Serialize::serialize(self.as_str(), s)
     }
 }
diff --git a/relay-base-schema/src/metrics/units.rs b/relay-base-schema/src/metrics/units.rs
@@ -123,15 +123,15 @@ impl IntoValue for MetricUnit {
     where
         Self: Sized,
     {
-        Value::String(format!("{self}"))
+        Value::String(self.to_string())
     }
 
     fn serialize_payload<S>(&self, s: S, _behavior: SkipSerialization) -> Result<S::Ok, S::Error>
     where
         Self: Sized,
         S: serde::Serializer,
     {
-        serde::Serialize::serialize(&self.to_string(), s)
+        serde::Serialize::serialize(self.as_str(), s)
     }
 }
 

diff --git a/relay-event-normalization/src/normalize/span/tag_extraction.rs b/relay-event-normalization/src/normalize/span/tag_extraction.rs
@@ -356,7 +356,7 @@ fn extract_shared_tags(event: &Event) -> BTreeMap<SpanTagKey, String> {
 
     if let Some(trace_context) = event.context::<TraceContext>() {
         if let Some(op) = extract_transaction_op(trace_context) {
-            tags.insert(SpanTagKey::TransactionOp, op.to_lowercase().to_owned());
+            tags.insert(SpanTagKey::TransactionOp, op.to_lowercase());
         }
 
         if let Some(status) = trace_context.status.value() {
@@ -527,7 +527,7 @@ pub fn extract_tags(
     }
 
     if let Some(unsanitized_span_op) = span.op.value() {
-        let span_op = unsanitized_span_op.to_owned().to_lowercase();
+        let span_op = unsanitized_span_op.to_lowercase();
 
         span_tags.insert(SpanTagKey::SpanOp, span_op.to_owned());
 

diff --git a/relay-event-schema/src/protocol/contexts/trace.rs b/relay-event-schema/src/protocol/contexts/trace.rs
@@ -13,13 +13,14 @@ pub struct TraceId(pub String);
 impl FromValue for TraceId {
     fn from_value(value: Annotated<Value>) -> Annotated<Self> {
         match value {
-            Annotated(Some(Value::String(value)), mut meta) => {
+            Annotated(Some(Value::String(mut value)), mut meta) => {
                 if !is_hex_string(&value, 32) || value.bytes().all(|x| x == b'0') {
                     meta.add_error(Error::invalid("not a valid trace id"));
                     meta.set_original_value(Some(value));
                     Annotated(None, meta)
                 } else {
-                    Annotated(Some(TraceId(value.to_ascii_lowercase())), meta)
+                    value.make_ascii_lowercase();
+                    Annotated(Some(TraceId(value)), meta)
                 }
             }
             Annotated(None, meta) => Annotated(None, meta),
@@ -61,13 +62,14 @@ impl fmt::Display for SpanId {
 impl FromValue for SpanId {
     fn from_value(value: Annotated<Value>) -> Annotated<Self> {
         match value {
-            Annotated(Some(Value::String(value)), mut meta) => {
+            Annotated(Some(Value::String(mut value)), mut meta) => {
                 if !is_hex_string(&value, 16) || value.bytes().all(|x| x == b'0') {
                     meta.add_error(Error::invalid("not a valid span id"));
                     meta.set_original_value(Some(value));
                     Annotated(None, meta)
                 } else {
-                    Annotated(Some(SpanId(value.to_ascii_lowercase())), meta)
+                    value.make_ascii_lowercase();
+                    Annotated(Some(SpanId(value)), meta)
                 }
             }
             Annotated(None, meta) => Annotated(None, meta),

diff --git a/relay-event-schema/src/protocol/event.rs b/relay-event-schema/src/protocol/event.rs
@@ -555,7 +555,7 @@ impl Event {
         for item in headers.iter() {
             if let Some((ref o_k, ref v)) = item.value() {
                 if let Some(k) = o_k.as_str() {
-                    if k.to_lowercase() == "user-agent" {
+                    if k.eq_ignore_ascii_case("user-agent") {
                         return v.as_str();
                     }
                 }

diff --git a/relay-event-schema/src/protocol/nel.rs b/relay-event-schema/src/protocol/nel.rs
@@ -71,11 +71,12 @@ impl FromStr for NetworkReportPhases {
     type Err = ParseNetworkReportPhaseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        Ok(match s.to_lowercase().as_str() {
+        let s = s.to_lowercase();
+        Ok(match s.as_str() {
             "dns" => NetworkReportPhases::DNS,
             "connection" => NetworkReportPhases::Connections,
             "application" => NetworkReportPhases::Application,
-            unknown => NetworkReportPhases::Other(unknown.to_string()),
+            _ => NetworkReportPhases::Other(s),
         })
     }
 }
@@ -103,7 +104,10 @@ impl FromValue for NetworkReportPhases {
 
 impl IntoValue for NetworkReportPhases {
     fn into_value(self) -> Value {
-        Value::String(self.to_string())
+        Value::String(match self {
+            Self::Other(s) => s,
+            _ => self.as_str().to_owned(),
+        })
     }
 
     fn serialize_payload<S>(
@@ -115,7 +119,7 @@ impl IntoValue for NetworkReportPhases {
         Self: Sized,
         S: serde::Serializer,
     {
-        Serialize::serialize(&self.to_string(), s)
+        Serialize::serialize(self.as_str(), s)
     }
 }
 

diff --git a/relay-event-schema/src/protocol/replay.rs b/relay-event-schema/src/protocol/replay.rs
@@ -235,7 +235,7 @@ impl Replay {
         for item in headers.iter() {
             if let Some((ref o_k, ref v)) = item.value() {
                 if let Some(k) = o_k.as_str() {
-                    if k.to_lowercase() == "user-agent" {
+                    if k.eq_ignore_ascii_case("user-agent") {
                         return v.as_str();
                     }
                 }

diff --git a/relay-event-schema/src/protocol/stacktrace.rs b/relay-event-schema/src/protocol/stacktrace.rs
@@ -397,6 +397,19 @@ pub enum InstructionAddrAdjustment {
     Unknown(String),
 }
 
+impl InstructionAddrAdjustment {
+    /// Returns the string representation of this adjustment.
+    pub fn as_str(&self) -> &str {
+        match self {
+            InstructionAddrAdjustment::Auto => "auto",
+            InstructionAddrAdjustment::AllButFirst => "all_but_first",
+            InstructionAddrAdjustment::All => "all",
+            InstructionAddrAdjustment::None => "none",
+            InstructionAddrAdjustment::Unknown(s) => s,
+        }
+    }
+}
+
 impl FromStr for InstructionAddrAdjustment {
     type Err = Infallible;
 
@@ -413,14 +426,7 @@ impl FromStr for InstructionAddrAdjustment {
 
 impl fmt::Display for InstructionAddrAdjustment {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let s = match self {
-            InstructionAddrAdjustment::Auto => "auto",
-            InstructionAddrAdjustment::AllButFirst => "all_but_first",
-            InstructionAddrAdjustment::All => "all",
-            InstructionAddrAdjustment::None => "none",
-            InstructionAddrAdjustment::Unknown(s) => s,
-        };
-        f.write_str(s)
+        f.write_str(self.as_str())
     }
 }
 
@@ -458,15 +464,18 @@ impl IntoValue for InstructionAddrAdjustment {
     where
         Self: Sized,
     {
-        Value::String(self.to_string())
+        Value::String(match self {
+            Self::Unknown(s) => s,
+            _ => self.as_str().to_owned(),
+        })
     }
 
     fn serialize_payload<S>(&self, s: S, _behavior: SkipSerialization) -> Result<S::Ok, S::Error>
     where
         Self: Sized,
         S: serde::Serializer,
     {
-        serde::Serialize::serialize(&self.to_string(), s)
+        serde::Serialize::serialize(self.as_str(), s)
     }
 }
 

diff --git a/relay-event-schema/src/protocol/transaction.rs b/relay-event-schema/src/protocol/transaction.rs
@@ -105,15 +105,18 @@ impl IntoValue for TransactionSource {
     where
         Self: Sized,
     {
-        Value::String(self.to_string())
+        Value::String(match self {
+            Self::Other(s) => s,
+            _ => self.as_str().to_owned(),
+        })
     }
 
     fn serialize_payload<S>(&self, s: S, _behavior: SkipSerialization) -> Result<S::Ok, S::Error>
     where
         Self: Sized,
         S: serde::Serializer,
     {
-        serde::Serialize::serialize(&self.to_string(), s)
+        serde::Serialize::serialize(self.as_str(), s)
     }
 }
 

diff --git a/relay-event-schema/src/protocol/types.rs b/relay-event-schema/src/protocol/types.rs
@@ -399,7 +399,7 @@ macro_rules! hex_metrastructure {
                 Self: Sized,
                 S: Serializer,
             {
-                Serialize::serialize(&self.to_string(), s)
+                Serializer::collect_str(s, self)
             }
         }
 
@@ -466,7 +466,7 @@ impl IpAddr {
 
     /// Checks whether the contained ip address is still valid (relevant for PII processing).
     pub fn is_valid(&self) -> bool {
-        Self::parse(&self.0).is_ok()
+        self.is_auto() || net::IpAddr::from_str(&self.0).is_ok()
     }
 
     /// Returns the string value of this ip address.
@@ -664,7 +664,7 @@ impl IntoValue for Level {
         Self: Sized,
         S: Serializer,
     {
-        Serialize::serialize(&self.to_string(), s)
+        Serialize::serialize(self.name(), s)
     }
 }
 

diff --git a/relay-pattern/src/lib.rs b/relay-pattern/src/lib.rs
@@ -26,7 +26,7 @@
 //! * `{a,b}` matches any pattern within the alternation group.
 //! * `\` escapes any of the above special characters and treats it as a literal.
 
-use std::fmt;
+use std::fmt::{self, Write};
 
 /// Pattern parsing error.
 #[derive(Debug)]
@@ -255,7 +255,7 @@ fn to_regex(tokens: &[Token], options: Options) -> Result<regex_lite::Regex, Err
                     i => {
                         re.push('.');
                         re.push('{');
-                        re.push_str(&i.to_string());
+                        write!(re, "{i}").ok();
                         re.push('}')
                     }
                 },

diff --git a/relay-pii/src/selector.rs b/relay-pii/src/selector.rs
@@ -146,7 +146,7 @@ impl SelectorPathItem {
             (SelectorPathItem::Key(ref key), _) => state
                 .path()
                 .key()
-                .map(|k| k.to_lowercase() == key.to_lowercase())
+                .map(|k| k.eq_ignore_ascii_case(key))
                 .unwrap_or(false),
         }
     }