Merge branch '1.14.x'

benchmarks/benchmarks-core/src/jmh/java/io/micrometer/benchmark/core/MeterRemovalBenchmark.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright 2024 VMware, Inc.
+ *
+ * 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
+ *
+ * https://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 io.micrometer.benchmark.core;
+
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
+import org.openjdk.jmh.annotations.*;
+import org.openjdk.jmh.profile.GCProfiler;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.RunnerException;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+import java.util.concurrent.TimeUnit;
+
+@State(Scope.Benchmark)
+@Fork(1)
+@Warmup(iterations = 2)
+@Measurement(iterations = 5)
+@OutputTimeUnit(TimeUnit.MICROSECONDS)
+public class MeterRemovalBenchmark {
+
+    public static void main(String[] args) throws RunnerException {
+        Options opt = new OptionsBuilder().include(MeterRemovalBenchmark.class.getSimpleName())
+            .addProfiler(GCProfiler.class)
+            .build();
+
+        new Runner(opt).run();
+    }
+
+    @Param({ "10000", "100000" })
+    int meterCount;
+
+    MeterRegistry registry = new SimpleMeterRegistry();
+
+    @Setup
+    public void setup() {
+        for (int i = 0; i < meterCount; i++) {
+            registry.counter("counter", "key", String.valueOf(i));
+        }
+    }
+
+    /**
+     * Benchmark the time to remove one meter from a registry with many meters. This uses
+     * the single shot mode because otherwise it would measure the time to remove a meter
+     * not in the registry after the first call, and that is not what we want to measure.
+     */
+    @Benchmark
+    @Warmup(iterations = 100)
+    @Measurement(iterations = 500)
+    @BenchmarkMode(Mode.SingleShotTime)
+    public Meter remove() {
+        return registry.remove(registry.counter("counter", "key", String.valueOf(meterCount / 2)));
+    }
+
+}

micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java

@@ -112,6 +112,12 @@ public abstract class MeterRegistry {
      */
     private final Map<Id, Meter> preFilterIdToMeterMap = new HashMap<>();
 
+    /**
+     * For reverse looking up pre-filter ID in {@link #preFilterIdToMeterMap} from the
+     * Meter being removed in {@link #remove(Id)}. Guarded by the {@link #meterMapLock}.
+     */
+    private final Map<Meter, Id> meterToPreFilterIdMap = new HashMap<>();
+
     /**
      * Only needed when MeterFilter configured after Meters registered. Write/remove
      * guarded by meterMapLock, remove in {@link #unmarkStaleId(Id)} and other operations
@@ -684,6 +690,7 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
                     }
                     meterMap.put(mappedId, m);
                     preFilterIdToMeterMap.put(originalId, m);
+                    meterToPreFilterIdMap.put(m, originalId);
                     unmarkStaleId(originalId);
                 }
             }
@@ -774,14 +781,9 @@ public Meter remove(Meter.Id mappedId) {
             synchronized (meterMapLock) {
                 final Meter removedMeter = meterMap.remove(mappedId);
                 if (removedMeter != null) {
-                    Iterator<Map.Entry<Id, Meter>> iterator = preFilterIdToMeterMap.entrySet().iterator();
-                    while (iterator.hasNext()) {
-                        Map.Entry<Id, Meter> nextEntry = iterator.next();
-                        if (nextEntry.getValue().equals(removedMeter)) {
-                            stalePreFilterIds.remove(nextEntry.getKey());
-                            iterator.remove();
-                        }
-                    }
+                    Id preFilterIdToRemove = meterToPreFilterIdMap.remove(removedMeter);
+                    preFilterIdToMeterMap.remove(preFilterIdToRemove);
+                    stalePreFilterIds.remove(preFilterIdToRemove);
 
                     Set<Id> synthetics = syntheticAssociations.remove(mappedId);
                     if (synthetics != null) {
@@ -1229,6 +1231,11 @@ Map<Id, Meter> _getPreFilterIdToMeterMap() {
         return Collections.unmodifiableMap(preFilterIdToMeterMap);
     }
 
+    // VisibleForTesting
+    Map<Meter, Id> _getMeterToPreFilterIdMap() {
+        return Collections.unmodifiableMap(meterToPreFilterIdMap);
+    }
+
     // VisibleForTesting
     Set<Id> _getStalePreFilterIds() {
         return Collections.unmodifiableSet(stalePreFilterIds);

micrometer-core/src/test/java/io/micrometer/core/instrument/MeterRegistryTest.java

@@ -287,10 +287,12 @@ void differentPreFilterIdsMapToSameId_thenCacheIsBounded() {
         // to the map because it would result in a memory leak with a high cardinality tag
         // that's otherwise limited in cardinality by a MeterFilter
         assertThat(registry._getPreFilterIdToMeterMap()).hasSize(1);
+        assertThat(registry._getMeterToPreFilterIdMap()).hasSize(1);
 
         assertThat(registry.remove(c1)).isSameAs(c2);
         assertThat(registry.getMeters()).isEmpty();
         assertThat(registry._getPreFilterIdToMeterMap()).isEmpty();
+        assertThat(registry._getMeterToPreFilterIdMap()).isEmpty();
     }
 
     @Test
@@ -318,6 +320,7 @@ void removingStaleMeterRemovesItFromAllInternalState() {
         registry.remove(c1.getId());
         assertThat(registry.getMeters()).isEmpty();
         assertThat(registry._getPreFilterIdToMeterMap()).isEmpty();
+        assertThat(registry._getMeterToPreFilterIdMap()).isEmpty();
         assertThat(registry._getStalePreFilterIds()).isEmpty();
     }
 
@@ -331,6 +334,8 @@ void multiplePreFilterIdsMapToSameId_removeByPreFilterId() {
         Meter.Id preFilterId = new Meter.Id("counter", Tags.of("secret", "value2"), null, null, Meter.Type.COUNTER);
         assertThat(registry.removeByPreFilterId(preFilterId)).isSameAs(c1).isSameAs(c2);
         assertThat(registry.getMeters()).isEmpty();
+        assertThat(registry._getPreFilterIdToMeterMap()).isEmpty();
+        assertThat(registry._getMeterToPreFilterIdMap()).isEmpty();
     }
 
     @Test