AttributeSet cleanup, better perf for overflows (#2313)

Author: cijothomas

opentelemetry-sdk/benches/metrics_counter.rs

@@ -9,7 +9,7 @@
     |--------------------------------|-------------|
     | Counter_Add_Sorted             | 172 ns      |
     | Counter_Add_Unsorted           | 183 ns      |
-    | Counter_Overflow               | 898 ns      |
+    | Counter_Overflow               | 562 ns      |
     | ThreadLocal_Random_Generator_5 |  37 ns      |
 */
 

opentelemetry-sdk/src/metrics/internal/mod.rs

@@ -18,8 +18,6 @@ pub(crate) use exponential_histogram::{EXPO_MAX_SCALE, EXPO_MIN_SCALE};
 use once_cell::sync::Lazy;
 use opentelemetry::{otel_warn, KeyValue};
 
-use crate::metrics::AttributeSet;
-
 pub(crate) static STREAM_OVERFLOW_ATTRIBUTES: Lazy<Vec<KeyValue>> =
     Lazy::new(|| vec![KeyValue::new("otel.metric.overflow", "true")]);
 
@@ -95,7 +93,7 @@ where
         }
 
         // Try to retrieve and update the tracker with the attributes sorted.
-        let sorted_attrs = AttributeSet::from(attributes).into_vec();
+        let sorted_attrs = sort_and_dedup(attributes);
         if let Some(tracker) = trackers.get(sorted_attrs.as_slice()) {
             tracker.update(value);
             return;
@@ -198,6 +196,16 @@ fn prepare_data<T>(data: &mut Vec<T>, list_len: usize) {
     }
 }
 
+fn sort_and_dedup(attributes: &[KeyValue]) -> Vec<KeyValue> {
+    // Use newly allocated vec here as incoming attributes are immutable so
+    // cannot sort/de-dup in-place. TODO: This allocation can be avoided by
+    // leveraging a ThreadLocal vec.
+    let mut sorted = attributes.to_vec();
+    sorted.sort_unstable_by(|a, b| a.key.cmp(&b.key));
+    sorted.dedup_by(|a, b| a.key == b.key);
+    sorted
+}
+
 /// Marks a type that can have a value added and retrieved atomically. Required since
 /// different types have different backing atomic mechanisms
 pub(crate) trait AtomicTracker<T>: Sync + Send + 'static {

opentelemetry-sdk/src/metrics/mod.rs

@@ -77,11 +77,7 @@ pub use view::*;
 // #[cfg(not(feature = "spec_unstable_metrics_views"))]
 // pub(crate) use view::*;
 
-use std::collections::hash_map::DefaultHasher;
-use std::collections::HashSet;
-use std::hash::{Hash, Hasher};
-
-use opentelemetry::KeyValue;
+use std::hash::Hash;
 
 /// Defines the window that an aggregation was calculated over.
 #[derive(Debug, Copy, Clone, Default, PartialEq, Eq, Hash)]
@@ -106,60 +102,6 @@ pub enum Temporality {
     LowMemory,
 }
 
-/// A unique set of attributes that can be used as instrument identifiers.
-///
-/// This must implement [Hash], [PartialEq], and [Eq] so it may be used as
-/// HashMap keys and other de-duplication methods.
-#[derive(Clone, Default, Debug, PartialEq, Eq)]
-pub(crate) struct AttributeSet(Vec<KeyValue>, u64);
-
-impl From<&[KeyValue]> for AttributeSet {
-    fn from(values: &[KeyValue]) -> Self {
-        let mut seen_keys = HashSet::with_capacity(values.len());
-        let vec = values
-            .iter()
-            .rev()
-            .filter_map(|kv| {
-                if seen_keys.insert(kv.key.clone()) {
-                    Some(kv.clone())
-                } else {
-                    None
-                }
-            })
-            .collect::<Vec<_>>();
-
-        AttributeSet::new(vec)
-    }
-}
-
-fn calculate_hash(values: &[KeyValue]) -> u64 {
-    let mut hasher = DefaultHasher::new();
-    values.iter().fold(&mut hasher, |mut hasher, item| {
-        item.hash(&mut hasher);
-        hasher
-    });
-    hasher.finish()
-}
-
-impl AttributeSet {
-    fn new(mut values: Vec<KeyValue>) -> Self {
-        values.sort_unstable_by(|a, b| a.key.cmp(&b.key));
-        let hash = calculate_hash(&values);
-        AttributeSet(values, hash)
-    }
-
-    /// Returns the underlying Vec of KeyValue pairs
-    pub(crate) fn into_vec(self) -> Vec<KeyValue> {
-        self.0
-    }
-}
-
-impl Hash for AttributeSet {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        state.write_u64(self.1)
-    }
-}
-
 #[cfg(all(test, feature = "testing"))]
 mod tests {
     use self::data::{DataPoint, HistogramDataPoint, ScopeMetrics};