Avoid intermediate string allocation for label values

Author: xudong963

datafusion/datasource-parquet/src/metrics.rs

@@ -15,6 +15,8 @@
 // specific language governing permissions and limitations
 // under the License.
 
+use std::sync::Arc;
+
 use datafusion_physical_plan::metrics::{
     Count, ExecutionPlanMetricsSet, Gauge, Label, MetricBuilder, MetricCategory,
     MetricType, PruningMetrics, RatioMergeStrategy, RatioMetrics, Time,
@@ -102,7 +104,7 @@ impl ParquetFileMetrics {
     ) -> Self {
         // Share the filename label across all per-file metrics to avoid
         // allocating the same filename string for each metric.
-        let filename_label = Label::new("filename", filename.to_string());
+        let filename_label = Label::new("filename", Arc::<str>::from(filename));
         let builder = MetricBuilder::new(metrics).with_label(filename_label);
 
         // -----------------------

datafusion/physical-expr-common/src/metrics/builder.rs

@@ -25,7 +25,8 @@ use crate::metrics::{
 };
 
 use super::{
-    Count, ExecutionPlanMetricsSet, Gauge, Label, Metric, MetricValue, Time, Timestamp,
+    Count, ExecutionPlanMetricsSet, Gauge, Label, LabelValue, Metric, MetricValue, Time,
+    Timestamp,
 };
 
 /// Structure for constructing metrics, counters, timers, etc.
@@ -110,7 +111,10 @@ impl<'a> MetricBuilder<'a> {
         name: impl Into<Cow<'static, str>>,
         value: impl Into<Cow<'static, str>>,
     ) -> Self {
-        self.with_label(Label::new(name.into(), value.into()))
+        self.with_label(Label::new(
+            LabelValue::from(name.into()),
+            LabelValue::from(value.into()),
+        ))
     }
 
     /// Set the partition of the metric being constructed

datafusion/physical-expr-common/src/metrics/mod.rs

@@ -532,12 +532,9 @@ pub struct Label {
 
 impl Label {
     /// Create a new [`Label`]
-    pub fn new(
-        name: impl Into<Cow<'static, str>>,
-        value: impl Into<Cow<'static, str>>,
-    ) -> Self {
-        let name = LabelValue::from(name.into());
-        let value = LabelValue::from(value.into());
+    pub fn new(name: impl Into<LabelValue>, value: impl Into<LabelValue>) -> Self {
+        let name = name.into();
+        let value = value.into();
         Self { name, value }
     }
 
@@ -558,32 +555,55 @@ impl Display for Label {
     }
 }
 
-/// Internal representation for label names and values.
+/// A label name or value.
 ///
-/// `Static` preserves the existing allocation-free path for string literals.
-/// `Shared` stores dynamic strings behind [`Arc<str>`], so cloning a [`Label`]
-/// only increments an atomic reference count and does not allocate or copy the
-/// underlying string data.
-#[derive(Clone, Eq)]
-enum LabelValue {
+/// String literals preserve the existing allocation-free path. Dynamic strings
+/// can be stored behind [`Arc<str>`], so cloning a [`Label`] only increments an
+/// atomic reference count and does not allocate or copy the underlying string
+/// data.
+#[derive(Clone)]
+pub struct LabelValue(LabelValueInner);
+
+/// Internal representation for label names and values.
+#[derive(Clone)]
+enum LabelValueInner {
     Static(&'static str),
     Shared(Arc<str>),
 }
 
 impl LabelValue {
-    fn as_str(&self) -> &str {
-        match self {
-            Self::Static(value) => value,
-            Self::Shared(value) => value.as_ref(),
+    /// Return this label value as a string slice.
+    pub fn as_str(&self) -> &str {
+        match &self.0 {
+            LabelValueInner::Static(value) => value,
+            LabelValueInner::Shared(value) => value.as_ref(),
         }
     }
 }
 
+impl From<&'static str> for LabelValue {
+    fn from(value: &'static str) -> Self {
+        Self(LabelValueInner::Static(value))
+    }
+}
+
+impl From<String> for LabelValue {
+    fn from(value: String) -> Self {
+        Self(LabelValueInner::Shared(Arc::from(value)))
+    }
+}
+
+impl From<Arc<str>> for LabelValue {
+    fn from(value: Arc<str>) -> Self {
+        Self(LabelValueInner::Shared(value))
+    }
+}
+
 impl From<Cow<'static, str>> for LabelValue {
     fn from(value: Cow<'static, str>) -> Self {
         match value {
-            Cow::Borrowed(value) => Self::Static(value),
-            Cow::Owned(value) => Self::Shared(Arc::from(value)),
+            Cow::Borrowed(value) => value.into(),
+            Cow::Owned(value) => value.into(),
         }
     }
 }
@@ -594,6 +614,8 @@ impl PartialEq for LabelValue {
     }
 }
 
+impl Eq for LabelValue {}
+
 impl Hash for LabelValue {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.as_str().hash(state);
@@ -670,9 +692,12 @@ mod tests {
     fn test_label_owned_and_borrowed_values_are_equal() {
         let borrowed = Label::new("foo", "bar");
         let owned = Label::new("foo".to_string(), "bar".to_string());
+        let shared = Label::new("foo", Arc::<str>::from("bar"));
 
         assert_eq!(borrowed, owned);
+        assert_eq!(borrowed, shared);
         assert_eq!(borrowed.to_string(), owned.to_string());
+        assert_eq!(borrowed.to_string(), shared.to_string());
     }
 
     #[test]