diff --git a/benchmarks/benchmarks-core/src/jmh/java/io/micrometer/benchmark/core/MeterRemovalBenchmark.java b/benchmarks/benchmarks-core/src/jmh/java/io/micrometer/benchmark/core/MeterRemovalBenchmark.java new file mode 100644 index 000000000..1f046bd92 --- /dev/null +++ b/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))); + } + +} diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java index 863c0631e..4e3a453ce 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java @@ -112,6 +112,12 @@ public abstract class MeterRegistry { */ private final Map 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 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> iterator = preFilterIdToMeterMap.entrySet().iterator(); - while (iterator.hasNext()) { - Map.Entry 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 synthetics = syntheticAssociations.remove(mappedId); if (synthetics != null) { @@ -1229,6 +1231,11 @@ Map _getPreFilterIdToMeterMap() { return Collections.unmodifiableMap(preFilterIdToMeterMap); } + // VisibleForTesting + Map _getMeterToPreFilterIdMap() { + return Collections.unmodifiableMap(meterToPreFilterIdMap); + } + // VisibleForTesting Set _getStalePreFilterIds() { return Collections.unmodifiableSet(stalePreFilterIds); diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/MeterRegistryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/MeterRegistryTest.java index 25246d662..678583b23 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/MeterRegistryTest.java +++ b/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