From 8c3c3c22aa99965f17048fd9303c7915b41595b4 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Wed, 11 Sep 2024 12:28:19 +0200 Subject: [PATCH] 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. --- relay-base-schema/src/events.rs | 4 +-- relay-base-schema/src/metrics/units.rs | 4 +-- .../src/normalize/span/tag_extraction.rs | 4 +-- .../src/protocol/contexts/trace.rs | 10 ++++--- relay-event-schema/src/protocol/event.rs | 2 +- relay-event-schema/src/protocol/nel.rs | 12 +++++--- relay-event-schema/src/protocol/replay.rs | 2 +- relay-event-schema/src/protocol/stacktrace.rs | 29 ++++++++++++------- .../src/protocol/transaction.rs | 7 +++-- relay-event-schema/src/protocol/types.rs | 6 ++-- relay-pattern/src/lib.rs | 4 +-- relay-pii/src/selector.rs | 2 +- 12 files changed, 52 insertions(+), 34 deletions(-) diff --git a/relay-base-schema/src/events.rs b/relay-base-schema/src/events.rs index 720cfa5be5..1c924b1374 100644 --- a/relay-base-schema/src/events.rs +++ b/relay-base-schema/src/events.rs @@ -132,7 +132,7 @@ impl IntoValue for EventType { where Self: Sized, { - Value::String(format!("{self}")) + Value::String(self.to_string()) } fn serialize_payload(&self, s: S, _behavior: SkipSerialization) -> Result @@ -140,6 +140,6 @@ impl IntoValue for EventType { 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 index e61ae40cc4..e85c9c98cb 100644 --- a/relay-base-schema/src/metrics/units.rs +++ b/relay-base-schema/src/metrics/units.rs @@ -123,7 +123,7 @@ impl IntoValue for MetricUnit { where Self: Sized, { - Value::String(format!("{self}")) + Value::String(self.to_string()) } fn serialize_payload(&self, s: S, _behavior: SkipSerialization) -> Result @@ -131,7 +131,7 @@ impl IntoValue for MetricUnit { 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 index ce00111c90..4b8a355f5c 100644 --- 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 { if let Some(trace_context) = event.context::() { 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 index a7a6e54f7c..8526fb76db 100644 --- 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) -> Annotated { 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) -> Annotated { 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 index 5fe1624183..42221e5ada 100644 --- 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 index cc5865ae1f..64bd1b02c1 100644 --- 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 { - 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( @@ -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 index 042647c6c6..9fc2bb97c3 100644 --- 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 index bd70313482..718ac7aceb 100644 --- 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,7 +464,10 @@ 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(&self, s: S, _behavior: SkipSerialization) -> Result @@ -466,7 +475,7 @@ impl IntoValue for InstructionAddrAdjustment { 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 index 58d572dda7..872c73cb0b 100644 --- a/relay-event-schema/src/protocol/transaction.rs +++ b/relay-event-schema/src/protocol/transaction.rs @@ -105,7 +105,10 @@ 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(&self, s: S, _behavior: SkipSerialization) -> Result @@ -113,7 +116,7 @@ impl IntoValue for TransactionSource { 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 index bd0ae7e156..8379c2be22 100644 --- 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 index 99a9e139f2..5dae4b9c16 100644 --- 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 { 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 index 4732249d4a..c0481107bf 100644 --- 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), } }