Box complex types in AnyValue enum (Improve perf by 10%, and size red…

…uction by ~60%) (#1993) Co-authored-by: Utkarsh Umesan Pillai <[email protected]> Co-authored-by: Cijo Thomas <[email protected]>
open-telemetry · Aug 8, 2024 · 2409c18 · 2409c18
1 parent fe10ab1
commit 2409c18
Show file tree

Hide file tree

Showing 6 changed files with 107 additions and 44 deletions.
diff --git a/opentelemetry-appender-log/src/lib.rs b/opentelemetry-appender-log/src/lib.rs
@@ -429,7 +429,7 @@ mod any_value {
         }
 
         fn serialize_bytes(self, v: &[u8]) -> Result<Self::Ok, Self::Error> {
-            Ok(Some(AnyValue::Bytes(v.to_owned())))
+            Ok(Some(AnyValue::Bytes(Box::new(v.to_owned()))))
         }
 
         fn serialize_none(self) -> Result<Self::Ok, Self::Error> {
@@ -557,7 +557,7 @@ mod any_value {
         }
 
         fn end(self) -> Result<Self::Ok, Self::Error> {
-            Ok(Some(AnyValue::ListAny(self.value)))
+            Ok(Some(AnyValue::ListAny(Box::new(self.value))))
         }
     }
 
@@ -578,7 +578,7 @@ mod any_value {
         }
 
         fn end(self) -> Result<Self::Ok, Self::Error> {
-            Ok(Some(AnyValue::ListAny(self.value)))
+            Ok(Some(AnyValue::ListAny(Box::new(self.value))))
         }
     }
 
@@ -599,7 +599,7 @@ mod any_value {
         }
 
         fn end(self) -> Result<Self::Ok, Self::Error> {
-            Ok(Some(AnyValue::ListAny(self.value)))
+            Ok(Some(AnyValue::ListAny(Box::new(self.value))))
         }
     }
 
@@ -621,8 +621,11 @@ mod any_value {
 
         fn end(self) -> Result<Self::Ok, Self::Error> {
             Ok(Some(AnyValue::Map({
-                let mut variant = HashMap::new();
-                variant.insert(Key::from(self.variant), AnyValue::ListAny(self.value));
+                let mut variant = Box::<HashMap<Key, AnyValue>>::default();
+                variant.insert(
+                    Key::from(self.variant),
+                    AnyValue::ListAny(Box::new(self.value)),
+                );
                 variant
             })))
         }
@@ -664,7 +667,7 @@ mod any_value {
         }
 
         fn end(self) -> Result<Self::Ok, Self::Error> {
-            Ok(Some(AnyValue::Map(self.value)))
+            Ok(Some(AnyValue::Map(Box::new(self.value))))
         }
     }
 
@@ -688,7 +691,7 @@ mod any_value {
         }
 
         fn end(self) -> Result<Self::Ok, Self::Error> {
-            Ok(Some(AnyValue::Map(self.value)))
+            Ok(Some(AnyValue::Map(Box::new(self.value))))
         }
     }
 
@@ -713,8 +716,8 @@ mod any_value {
 
         fn end(self) -> Result<Self::Ok, Self::Error> {
             Ok(Some(AnyValue::Map({
-                let mut variant = HashMap::new();
-                variant.insert(Key::from(self.variant), AnyValue::Map(self.value));
+                let mut variant = Box::<HashMap<Key, AnyValue>>::default();
+                variant.insert(Key::from(self.variant), AnyValue::Map(Box::new(self.value)));
                 variant
             })))
         }
@@ -1025,13 +1028,17 @@ mod tests {
             assert_eq!(AnyValue::Int(42), get("some_value").unwrap());
 
             assert_eq!(
-                AnyValue::ListAny(vec![AnyValue::Int(1), AnyValue::Int(1), AnyValue::Int(1)]),
+                AnyValue::ListAny(Box::new(vec![
+                    AnyValue::Int(1),
+                    AnyValue::Int(1),
+                    AnyValue::Int(1)
+                ])),
                 get("slice_value").unwrap()
             );
 
             assert_eq!(
                 AnyValue::Map({
-                    let mut map = HashMap::new();
+                    let mut map = Box::<HashMap<Key, AnyValue>>::default();
 
                     map.insert(Key::from("a"), AnyValue::Int(1));
                     map.insert(Key::from("b"), AnyValue::Int(1));
@@ -1044,7 +1051,7 @@ mod tests {
 
             assert_eq!(
                 AnyValue::Map({
-                    let mut map = HashMap::new();
+                    let mut map = Box::<HashMap<Key, AnyValue>>::default();
 
                     map.insert(Key::from("a"), AnyValue::Int(1));
                     map.insert(Key::from("b"), AnyValue::Int(1));
@@ -1056,7 +1063,11 @@ mod tests {
             );
 
             assert_eq!(
-                AnyValue::ListAny(vec![AnyValue::Int(1), AnyValue::Int(1), AnyValue::Int(1)]),
+                AnyValue::ListAny(Box::new(vec![
+                    AnyValue::Int(1),
+                    AnyValue::Int(1),
+                    AnyValue::Int(1)
+                ])),
                 get("tuple_value").unwrap()
             );
 
@@ -1071,7 +1082,7 @@ mod tests {
 
                     map.insert(Key::from("Newtype"), AnyValue::Int(42));
 
-                    map
+                    Box::new(map)
                 }),
                 get("newtype_variant_value").unwrap()
             );
@@ -1082,18 +1093,16 @@ mod tests {
 
                     map.insert(
                         Key::from("Struct"),
-                        AnyValue::Map({
+                        AnyValue::Map(Box::new({
                             let mut map = HashMap::new();
-
                             map.insert(Key::from("a"), AnyValue::Int(1));
                             map.insert(Key::from("b"), AnyValue::Int(1));
                             map.insert(Key::from("c"), AnyValue::Int(1));
-
                             map
-                        }),
+                        })),
                     );
 
-                    map
+                    Box::new(map)
                 }),
                 get("struct_variant_value").unwrap()
             );
@@ -1104,14 +1113,14 @@ mod tests {
 
                     map.insert(
                         Key::from("Tuple"),
-                        AnyValue::ListAny(vec![
+                        AnyValue::ListAny(Box::new(vec![
                             AnyValue::Int(1),
                             AnyValue::Int(1),
                             AnyValue::Int(1),
-                        ]),
+                        ])),
                     );
 
-                    map
+                    Box::new(map)
                 }),
                 get("tuple_variant_value").unwrap()
             );

diff --git a/opentelemetry-proto/src/transform/logs.rs b/opentelemetry-proto/src/transform/logs.rs
@@ -49,7 +49,7 @@ pub mod tonic {
                         })
                         .collect(),
                 }),
-                LogsAnyValue::Bytes(v) => Value::BytesValue(v),
+                LogsAnyValue::Bytes(v) => Value::BytesValue(*v),
             }
         }
     }

diff --git a/opentelemetry-sdk/benches/log.rs b/opentelemetry-sdk/benches/log.rs
@@ -151,74 +151,82 @@ fn criterion_benchmark(c: &mut Criterion) {
         logger.emit(log_record);
     });
 
-    let bytes = AnyValue::Bytes(vec![25u8, 30u8, 40u8]);
+    let bytes = AnyValue::Bytes(Box::new(vec![25u8, 30u8, 40u8]));
     log_benchmark_group(c, "simple-log-with-bytes", |logger| {
         let mut log_record = logger.create_log_record();
         log_record.set_body("simple log".into());
         log_record.add_attribute("testbytes", bytes.clone());
         logger.emit(log_record);
     });
 
-    let bytes = AnyValue::Bytes(vec![
+    let bytes = AnyValue::Bytes(Box::new(vec![
         25u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8,
         30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8,
         40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8,
         30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8,
         40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8,
         30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8, 30u8, 40u8,
-    ]);
+    ]));
     log_benchmark_group(c, "simple-log-with-a-lot-of-bytes", |logger| {
         let mut log_record = logger.create_log_record();
         log_record.set_body("simple log".into());
         log_record.add_attribute("testbytes", bytes.clone());
         logger.emit(log_record);
     });
 
-    let vec_any_values = AnyValue::ListAny(vec![AnyValue::Int(25), "test".into(), true.into()]);
+    let vec_any_values = AnyValue::ListAny(Box::new(vec![
+        AnyValue::Int(25),
+        "test".into(),
+        true.into(),
+    ]));
     log_benchmark_group(c, "simple-log-with-vec-any-value", |logger| {
         let mut log_record = logger.create_log_record();
         log_record.set_body("simple log".into());
         log_record.add_attribute("testvec", vec_any_values.clone());
         logger.emit(log_record);
     });
 
-    let vec_any_values = AnyValue::ListAny(vec![AnyValue::Int(25), "test".into(), true.into()]);
-    let vec_any_values = AnyValue::ListAny(vec![
+    let vec_any_values = AnyValue::ListAny(Box::new(vec![
+        AnyValue::Int(25),
+        "test".into(),
+        true.into(),
+    ]));
+    let vec_any_values = AnyValue::ListAny(Box::new(vec![
         AnyValue::Int(25),
         "test".into(),
         true.into(),
         vec_any_values,
-    ]);
+    ]));
     log_benchmark_group(c, "simple-log-with-inner-vec-any-value", |logger| {
         let mut log_record = logger.create_log_record();
         log_record.set_body("simple log".into());
         log_record.add_attribute("testvec", vec_any_values.clone());
         logger.emit(log_record);
     });
 
-    let map_any_values = AnyValue::Map(HashMap::from([
+    let map_any_values = AnyValue::Map(Box::new(HashMap::from([
         ("testint".into(), 2.into()),
         ("testdouble".into(), 2.2.into()),
         ("teststring".into(), "test".into()),
-    ]));
+    ])));
     log_benchmark_group(c, "simple-log-with-map-any-value", |logger| {
         let mut log_record = logger.create_log_record();
         log_record.set_body("simple log".into());
         log_record.add_attribute("testmap", map_any_values.clone());
         logger.emit(log_record);
     });
 
-    let map_any_values = AnyValue::Map(HashMap::from([
+    let map_any_values = AnyValue::Map(Box::new(HashMap::from([
         ("testint".into(), 2.into()),
         ("testdouble".into(), 2.2.into()),
         ("teststring".into(), "test".into()),
-    ]));
-    let map_any_values = AnyValue::Map(HashMap::from([
+    ])));
+    let map_any_values = AnyValue::Map(Box::new(HashMap::from([
         ("testint".into(), 2.into()),
         ("testdouble".into(), 2.2.into()),
         ("teststring".into(), "test".into()),
         ("testmap".into(), map_any_values),
-    ]));
+    ])));
     log_benchmark_group(c, "simple-log-with-inner-map-any-value", |logger| {
         let mut log_record = logger.create_log_record();
         log_record.set_body("simple log".into());

diff --git a/opentelemetry-stdout/src/common.rs b/opentelemetry-stdout/src/common.rs
@@ -168,7 +168,7 @@ impl From<opentelemetry::logs::AnyValue> for Value {
                     })
                     .collect(),
             ),
-            opentelemetry::logs::AnyValue::Bytes(b) => Value::BytesValue(b),
+            opentelemetry::logs::AnyValue::Bytes(b) => Value::BytesValue(*b),
         }
     }
 }

diff --git a/opentelemetry/CHANGELOG.md b/opentelemetry/CHANGELOG.md
@@ -2,6 +2,52 @@
 
 ## vNext
 
+- **BREAKING** [#1993](https://github.com/open-telemetry/opentelemetry-rust/pull/1993) Box complex types in AnyValue enum
+Before:
+```rust
+#[derive(Debug, Clone, PartialEq)]
+pub enum AnyValue {
+    /// An integer value
+    Int(i64),
+    /// A double value
+    Double(f64),
+    /// A string value
+    String(StringValue),
+    /// A boolean value
+    Boolean(bool),
+    /// A byte array
+    Bytes(Vec<u8>),
+    /// An array of `Any` values
+    ListAny(Vec<AnyValue>),
+    /// A map of string keys to `Any` values, arbitrarily nested.
+    Map(HashMap<Key, AnyValue>),
+}
+```
+
+After:
+```rust
+#[derive(Debug, Clone, PartialEq)]
+pub enum AnyValue {
+    /// An integer value
+    Int(i64),
+    /// A double value
+    Double(f64),
+    /// A string value
+    String(StringValue),
+    /// A boolean value
+    Boolean(bool),
+    /// A byte array
+    Bytes(Box<Vec<u8>>),
+    /// An array of `Any` values
+    ListAny(Box<Vec<AnyValue>>),
+    /// A map of string keys to `Any` values, arbitrarily nested.
+    Map(Box<HashMap<Key, AnyValue>>),
+}
+```
+So the custom log appenders should box these types while adding them in message body, or
+attribute values. Similarly, the custom exporters should dereference these complex type values
+before serializing.
+
 ## v0.24.0
 
 - Add "metrics", "logs" to default features. With this, default feature list is

diff --git a/opentelemetry/src/logs/record.rs b/opentelemetry/src/logs/record.rs
@@ -59,11 +59,11 @@ pub enum AnyValue {
     /// A boolean value
     Boolean(bool),
     /// A byte array
-    Bytes(Vec<u8>),
+    Bytes(Box<Vec<u8>>),
     /// An array of `Any` values
-    ListAny(Vec<AnyValue>),
+    ListAny(Box<Vec<AnyValue>>),
     /// A map of string keys to `Any` values, arbitrarily nested.
-    Map(HashMap<Key, AnyValue>),
+    Map(Box<HashMap<Key, AnyValue>>),
 }
 
 macro_rules! impl_trivial_from {
@@ -98,17 +98,17 @@ impl_trivial_from!(bool, AnyValue::Boolean);
 impl<T: Into<AnyValue>> FromIterator<T> for AnyValue {
     /// Creates an [`AnyValue::ListAny`] value from a sequence of `Into<AnyValue>` values.
     fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
-        AnyValue::ListAny(iter.into_iter().map(Into::into).collect())
+        AnyValue::ListAny(Box::new(iter.into_iter().map(Into::into).collect()))
     }
 }
 
 impl<K: Into<Key>, V: Into<AnyValue>> FromIterator<(K, V)> for AnyValue {
     /// Creates an [`AnyValue::Map`] value from a sequence of key-value pairs
     /// that can be converted into a `Key` and `AnyValue` respectively.
     fn from_iter<I: IntoIterator<Item = (K, V)>>(iter: I) -> Self {
-        AnyValue::Map(HashMap::from_iter(
+        AnyValue::Map(Box::new(HashMap::from_iter(
             iter.into_iter().map(|(k, v)| (k.into(), v.into())),
-        ))
+        )))
     }
 }