tracing-context: optimize Visit impl (metrics-rs#94)

* tracing-context: optimize Visit impl

.github/workflows/ci.yml

@@ -24,7 +24,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        rust_version: ['1.36.0', 'stable', 'nightly']
+        rust_version: ['1.42.0', 'stable', 'nightly']
         os: [ubuntu-latest, windows-latest, macOS-latest]
     steps:
     - uses: actions/checkout@v1

metrics-tracing-context/.gitignore

@@ -0,0 +1,3 @@
+/target
+**/*.rs.bk
+Cargo.lock

metrics-tracing-context/Cargo.toml

@@ -15,12 +15,21 @@ readme = "README.md"
 categories = ["development-tools::debugging"]
 keywords = ["metrics", "tracing"]
 
+[lib]
+bench = false
+
+[[bench]]
+name = "visit"
+harness = false
+
 [dependencies]
 metrics = { version = "0.13.0-alpha.1", path = "../metrics", features = ["std"] }
 metrics-util = { version = "0.4.0-alpha.1", path = "../metrics-util" }
 tracing = "0.1"
 tracing-core = "0.1"
 tracing-subscriber = "0.2"
+itoa = "0.4"
 
 [dev-dependencies]
+criterion = "0.3"
 parking_lot = "0.11"

metrics-tracing-context/benches/visit.rs

@@ -0,0 +1,183 @@
+use criterion::{criterion_group, criterion_main, BatchSize, Benchmark, Criterion};
+use metrics_tracing_context::Labels;
+use tracing::Metadata;
+use tracing_core::{
+    field::Visit,
+    metadata,
+    metadata::{Kind, Level},
+    Callsite, Interest,
+};
+
+const BATCH_SIZE: usize = 1000;
+
+static CALLSITE: TestCallsite = TestCallsite;
+static CALLSITE_METADATA: Metadata = metadata! {
+    name: "test",
+    target: module_path!(),
+    level: Level::DEBUG,
+    fields: &["test"],
+    callsite: &CALLSITE,
+    kind: Kind::SPAN,
+};
+
+fn visit_benchmark(c: &mut Criterion) {
+    c.bench(
+        "visit",
+        Benchmark::new("record_str", |b| {
+            let field = CALLSITE
+                .metadata()
+                .fields()
+                .field("test")
+                .expect("test field missing");
+            b.iter_batched_ref(
+                || Labels(Vec::with_capacity(BATCH_SIZE)),
+                |labels| {
+                    labels.record_str(&field, "test test");
+                },
+                BatchSize::NumIterations(BATCH_SIZE as u64),
+            )
+        })
+        .with_function("record_bool[true]", |b| {
+            let field = CALLSITE
+                .metadata()
+                .fields()
+                .field("test")
+                .expect("test field missing");
+            b.iter_batched_ref(
+                || Labels(Vec::with_capacity(BATCH_SIZE)),
+                |labels| {
+                    labels.record_bool(&field, true);
+                },
+                BatchSize::NumIterations(BATCH_SIZE as u64),
+            )
+        })
+        .with_function("record_bool[false]", |b| {
+            let field = CALLSITE
+                .metadata()
+                .fields()
+                .field("test")
+                .expect("test field missing");
+            b.iter_batched_ref(
+                || Labels(Vec::with_capacity(BATCH_SIZE)),
+                |labels| {
+                    labels.record_bool(&field, false);
+                },
+                BatchSize::NumIterations(BATCH_SIZE as u64),
+            )
+        })
+        .with_function("record_i64", |b| {
+            let field = CALLSITE
+                .metadata()
+                .fields()
+                .field("test")
+                .expect("test field missing");
+            b.iter_batched_ref(
+                || Labels(Vec::with_capacity(BATCH_SIZE)),
+                |labels| {
+                    labels.record_i64(&field, -3423432);
+                },
+                BatchSize::NumIterations(BATCH_SIZE as u64),
+            )
+        })
+        .with_function("record_u64", |b| {
+            let field = CALLSITE
+                .metadata()
+                .fields()
+                .field("test")
+                .expect("test field missing");
+            b.iter_batched_ref(
+                || Labels(Vec::with_capacity(BATCH_SIZE)),
+                |labels| {
+                    labels.record_u64(&field, 3423432);
+                },
+                BatchSize::NumIterations(BATCH_SIZE as u64),
+            )
+        })
+        .with_function("record_debug", |b| {
+            let debug_struct = DebugStruct::new();
+            let field = CALLSITE
+                .metadata()
+                .fields()
+                .field("test")
+                .expect("test field missing");
+            b.iter_batched_ref(
+                || Labels(Vec::with_capacity(BATCH_SIZE)),
+                |labels| {
+                    labels.record_debug(&field, &debug_struct);
+                },
+                BatchSize::NumIterations(BATCH_SIZE as u64),
+            )
+        })
+        .with_function("record_debug[bool]", |b| {
+            let field = CALLSITE
+                .metadata()
+                .fields()
+                .field("test")
+                .expect("test field missing");
+            b.iter_batched_ref(
+                || Labels(Vec::with_capacity(BATCH_SIZE)),
+                |labels| {
+                    labels.record_debug(&field, &true);
+                },
+                BatchSize::NumIterations(BATCH_SIZE as u64),
+            )
+        })
+        .with_function("record_debug[i64]", |b| {
+            let value: i64 = -3423432;
+            let field = CALLSITE
+                .metadata()
+                .fields()
+                .field("test")
+                .expect("test field missing");
+            b.iter_batched_ref(
+                || Labels(Vec::with_capacity(BATCH_SIZE)),
+                |labels| {
+                    labels.record_debug(&field, &value);
+                },
+                BatchSize::NumIterations(BATCH_SIZE as u64),
+            )
+        })
+        .with_function("record_debug[u64]", |b| {
+            let value: u64 = 3423432;
+            let field = CALLSITE
+                .metadata()
+                .fields()
+                .field("test")
+                .expect("test field missing");
+            b.iter_batched_ref(
+                || Labels(Vec::with_capacity(BATCH_SIZE)),
+                |labels| {
+                    labels.record_debug(&field, &value);
+                },
+                BatchSize::NumIterations(BATCH_SIZE as u64),
+            )
+        }),
+    );
+}
+
+#[derive(Debug)]
+struct DebugStruct {
+    field1: String,
+    field2: u64,
+}
+
+impl DebugStruct {
+    pub fn new() -> DebugStruct {
+        DebugStruct {
+            field1: format!("yeehaw!"),
+            field2: 324242343243,
+        }
+    }
+}
+
+struct TestCallsite;
+
+impl Callsite for TestCallsite {
+    fn set_interest(&self, _interest: Interest) {}
+    fn metadata(&self) -> &Metadata<'_> {
+        &CALLSITE_METADATA
+    }
+}
+
+criterion_group!(benches, visit_benchmark);
+criterion_main!(benches);

metrics-tracing-context/src/lib.rs

@@ -63,7 +63,7 @@ pub mod label_filter;
 mod tracing_integration;
 
 pub use label_filter::LabelFilter;
-pub use tracing_integration::{MetricsLayer, SpanExt};
+pub use tracing_integration::{Labels, MetricsLayer, SpanExt};
 
 /// [`TracingContextLayer`] provides an implementation of a [`metrics::Layer`]
 /// for [`TracingContext`].

metrics-tracing-context/src/tracing_integration.rs

@@ -6,14 +6,40 @@ use tracing_core::span::{Attributes, Id, Record};
 use tracing_core::{field::Visit, Dispatch, Field, Subscriber};
 use tracing_subscriber::{layer::Context, registry::LookupSpan, Layer};
 
-struct Labels(Vec<Label>);
+/// Per-span extension for collecting labels from fields.
+///
+/// Hidden from documentation as there is no need for end users to ever touch this type, but it must
+/// be public in order to be pulled in by external benchmark code.
+#[doc(hidden)]
+pub struct Labels(pub Vec<Label>);
 
 impl Visit for Labels {
     fn record_str(&mut self, field: &Field, value: &str) {
         let label = Label::new(field.name(), value.to_owned());
         self.0.push(label);
     }
 
+    fn record_bool(&mut self, field: &Field, value: bool) {
+        let label = Label::new(field.name(), if value { "true" } else { "false " });
+        self.0.push(label);
+    }
+
+    fn record_i64(&mut self, field: &Field, value: i64) {
+        // Maximum length is 20 characters but 32 is a nice power-of-two number.
+        let mut s = String::with_capacity(32);
+        itoa::fmt(&mut s, value).expect("failed to format/write i64");
+        let label = Label::new(field.name(), s);
+        self.0.push(label);
+    }
+
+    fn record_u64(&mut self, field: &Field, value: u64) {
+        // Maximum length is 20 characters but 32 is a nice power-of-two number.
+        let mut s = String::with_capacity(32);
+        itoa::fmt(&mut s, value).expect("failed to format/write u64");
+        let label = Label::new(field.name(), s);
+        self.0.push(label);
+    }
+
     fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) {
         let value_string = format!("{:?}", value);
         let label = Label::new(field.name(), value_string);

metrics-util/benches/bucket.rs

@@ -1,10 +1,5 @@
-#[macro_use]
-extern crate criterion;
-
-#[macro_use]
-extern crate lazy_static;
-
-use criterion::{Benchmark, Criterion, Throughput};
+use criterion::{criterion_group, criterion_main, Benchmark, Criterion, Throughput};
+use lazy_static::lazy_static;
 use metrics_util::AtomicBucket;
 
 lazy_static! {

metrics-util/benches/registry.rs

@@ -1,7 +1,4 @@
-#[macro_use]
-extern crate criterion;
-
-use criterion::{BatchSize, Benchmark, Criterion};
+use criterion::{criterion_group, criterion_main, BatchSize, Benchmark, Criterion};
 use metrics::{Key, KeyData, Label, OnceKeyData};
 use metrics_util::Registry;
 

metrics-util/benches/streaming_integers.rs

@@ -1,10 +1,5 @@
-#[macro_use]
-extern crate criterion;
-
-#[macro_use]
-extern crate lazy_static;
-
-use criterion::{Benchmark, Criterion, Throughput};
+use criterion::{criterion_group, criterion_main, Benchmark, Criterion, Throughput};
+use lazy_static::lazy_static;
 use metrics_util::StreamingIntegers;
 use rand::{distributions::Distribution, rngs::SmallRng, SeedableRng};
 use rand_distr::Gamma;