Nested metric registries don't cause a sortedmap creation (#430)

palantir · Sep 18, 2019 · 980cac9 · 980cac9
1 parent 8c6b397
commit 980cac9
Show file tree

Hide file tree

Showing 16 changed files with 596 additions and 15 deletions.
diff --git a/.baseline/checkstyle/checkstyle.xml b/.baseline/checkstyle/checkstyle.xml
@@ -469,7 +469,7 @@
         <module name="NestedTryDepth"/> <!-- Java Coding Guide: Try/catch blocks: never nested -->
         <module name="NonEmptyAtclauseDescription"/> <!-- Java Style Guide: At-clauses -->
         <module name="ParameterName"> <!-- Java Style Guide: Parameter names -->
-            <property name="format" value="^[a-z][a-zA-Z0-9]+$"/>
+            <property name="format" value="^_?[a-z][a-zA-Z0-9]+$"/>
             <message key="name.invalidPattern" value="Parameter name ''{0}'' must match pattern ''{1}''."/>
         </module>
         <module name="SingleLineJavadoc"/> <!-- Java Style Guide: General form -->

diff --git a/.gitignore b/.gitignore
@@ -32,3 +32,6 @@ out/
 
 # Gc logs
 build*.gc.log
+
+# Jqwik property based testing
+.jqwik-database
diff --git a/build.gradle b/build.gradle
@@ -14,7 +14,7 @@ buildscript {
         classpath 'com.jfrog.bintray.gradle:gradle-bintray-plugin:1.8.4'
         classpath 'com.netflix.nebula:gradle-info-plugin:5.0.3'
         classpath 'com.netflix.nebula:nebula-publishing-plugin:13.5.1'
-        classpath 'com.palantir.baseline:gradle-baseline-java:2.6.0'
+        classpath 'com.palantir.baseline:gradle-baseline-java:2.7.0'
         classpath 'com.palantir.gradle.consistentversions:gradle-consistent-versions:1.12.4'
         classpath 'com.palantir.gradle.gitversion:gradle-git-version:0.12.2'
         classpath 'gradle.plugin.org.inferred:gradle-processors:3.1.0'
@@ -127,7 +127,9 @@ allprojects {
     })
 
     tasks.withType(Test) {
-        useJUnitPlatform()
+        useJUnitPlatform {
+            includeEngines 'jqwik'
+        }
         maxParallelForks Runtime.getRuntime().availableProcessors()
         testLogging {
             events "passed", "skipped", "failed"

diff --git a/changelog/@unreleased/pr-430.v2.yml b/changelog/@unreleased/pr-430.v2.yml
@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Nested metric registries cause fewer allocations.
+  links:
+  - https://github.com/palantir/tritium/pull/430
diff --git a/tritium-jmh/src/jmh/java/com/palantir/tritium/microbenchmarks/NestedMetricsBenchmark.java b/tritium-jmh/src/jmh/java/com/palantir/tritium/microbenchmarks/NestedMetricsBenchmark.java
@@ -0,0 +1,100 @@
+/*
+ * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.palantir.tritium.microbenchmarks;
+
+import com.codahale.metrics.Meter;
+import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry;
+import com.palantir.tritium.metrics.registry.MetricName;
+import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
+import java.util.concurrent.TimeUnit;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+import org.openjdk.jmh.profile.GCProfiler;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
+@Fork(1)
+@State(Scope.Benchmark)
+@SuppressWarnings({"designforextension", "NullAway"})
+public class NestedMetricsBenchmark {
+    private TaggedMetricRegistry metrics;
+
+    @Setup
+    public void before() {
+        metrics = constructBaseRegistry();
+    }
+
+    @Benchmark
+    public void benchmarkGetMetrics(Blackhole blackhole) {
+        metrics.getMetrics().forEach((name, metric) -> {
+            blackhole.consume(name);
+            blackhole.consume(metric);
+        });
+    }
+
+    @Benchmark
+    public void benchmarkForEach(Blackhole blackhole) {
+        metrics.forEachMetric((name, metric) -> {
+            blackhole.consume(name);
+            blackhole.consume(metric);
+        });
+    }
+
+    private static TaggedMetricRegistry constructBaseRegistry() {
+        TaggedMetricRegistry registry = new DefaultTaggedMetricRegistry();
+        for (int i = 0; i < 50; i++) {
+            registry.addMetrics("registry id", Integer.toString(i), constructSubRegistry());
+        }
+        return registry;
+    }
+
+    private static TaggedMetricRegistry constructSubRegistry() {
+        TaggedMetricRegistry registry = new DefaultTaggedMetricRegistry();
+        for (int i = 0; i < 100; i++) {
+            Meter meter = registry.meter(MetricName.builder()
+                    .safeName("some metric " + i)
+                    .putSafeTags("some tag", "some tag value")
+                    .build());
+            for (int j = 0; j < 1000; j++) {
+                meter.mark();
+            }
+        }
+        return registry;
+    }
+
+    public static void main(String[] _args) throws Exception {
+        Options opt = new OptionsBuilder()
+                .include(NestedMetricsBenchmark.class.getSimpleName())
+                .addProfiler(GCProfiler.class)
+                .build();
+        new Runner(opt).run();
+    }
+}
diff --git a/tritium-registry/build.gradle b/tritium-registry/build.gradle
@@ -20,6 +20,7 @@ dependencies {
     testImplementation 'org.junit.jupiter:junit-jupiter-params'
     testImplementation 'org.mockito:mockito-core'
     testImplementation 'org.mockito:mockito-junit-jupiter'
+    testImplementation 'net.jqwik:jqwik'
 
     // Work around https://github.com/immutables/immutables/issues/291
     compileOnly 'org.immutables:value::annotations'

diff --git a/...try/src/main/java/com/palantir/tritium/metrics/registry/AbstractTaggedMetricRegistry.java b/...try/src/main/java/com/palantir/tritium/metrics/registry/AbstractTaggedMetricRegistry.java
@@ -32,6 +32,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiConsumer;
 import java.util.function.Supplier;
 import javax.annotation.Nonnull;
 
@@ -152,15 +153,20 @@ public final Map<MetricName, Metric> getMetrics() {
         result.putAll(registry);
         taggedRegistries.forEach((tag, metrics) -> metrics.getMetrics()
                 .forEach((metricName, metric) -> result.put(
-                        MetricName.builder()
-                                .from(metricName)
-                                .putSafeTags(tag.getKey(), tag.getValue())
-                                .build(),
-                        metric)));
+                        RealMetricName.create(metricName, tag.getKey(), tag.getValue()), metric)));
 
         return result.build();
     }
 
+    @Override
+    public final void forEachMetric(BiConsumer<MetricName, Metric> consumer) {
+        registry.forEach(consumer);
+        taggedRegistries.forEach((tag, metrics) -> metrics.forEachMetric((metricName, metric) ->
+                consumer.accept(
+                        RealMetricName.create(metricName, tag.getKey(), tag.getValue()),
+                        metric)));
+    }
+
     @Override
     public final Optional<Metric> remove(MetricName metricName) {
         return Optional.ofNullable(registry.remove(metricName));

diff --git a/...gistry/src/main/java/com/palantir/tritium/metrics/registry/DropwizardTaggedMetricSet.java b/...gistry/src/main/java/com/palantir/tritium/metrics/registry/DropwizardTaggedMetricSet.java
@@ -21,6 +21,7 @@
 import com.codahale.metrics.Metric;
 import com.codahale.metrics.MetricSet;
 import java.util.Map;
+import java.util.function.BiConsumer;
 
 public final class DropwizardTaggedMetricSet implements TaggedMetricSet {
     private final MetricSet metricSet;
@@ -32,6 +33,11 @@ public DropwizardTaggedMetricSet(MetricSet metricSet) {
     @Override
     public Map<MetricName, Metric> getMetrics() {
         return metricSet.getMetrics().entrySet().stream()
-                .collect(toMap(entry -> MetricName.builder().safeName(entry.getKey()).build(), Map.Entry::getValue));
+                .collect(toMap(entry -> RealMetricName.create(entry.getKey()), Map.Entry::getValue));
+    }
+
+    @Override
+    public void forEachMetric(BiConsumer<MetricName, Metric> consumer) {
+        metricSet.getMetrics().forEach((name, metric) -> consumer.accept(RealMetricName.create(name), metric));
     }
 }