Skip to content

Commit d883ba9

Browse files
authored
Avoid capturing lambda allocation when retrieving existing meters (#6670)
In particular focuses on Timer, LongTaskTimer, and Counter as these are the types expected to be retrieved (non-functional) the most on hot code paths. Avoiding this allocation makes the retrieval faster and generates less garbage, causing less GC pressure. Adds to MeterRegistry new public overloads of timer, longTaskTimer, and counter methods that take `Tags` because the existing methods take `Iterable<Tag>` which means `Tags.of` needs to be called on it, which can result in allocation (to check the size of the Iterable) before JIT optimization. Adds tests to verify the expected allocation behavior of these methods to prevent regressions. It can be particularly difficult to notice allocation caused by capturing lambdas with code review alone.
1 parent 617debb commit d883ba9

File tree

6 files changed

+268
-43
lines changed

6 files changed

+268
-43
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
@Warmup(iterations = 2)
3434
@Measurement(iterations = 5)
3535
@BenchmarkMode(Mode.AverageTime)
36-
@OutputTimeUnit(TimeUnit.MICROSECONDS)
36+
@OutputTimeUnit(TimeUnit.NANOSECONDS)
3737
@Threads(2)
3838
public class MeterRegistrationBenchmark {
3939

@@ -49,7 +49,7 @@ public static void main(String[] args) throws RunnerException {
4949

5050
Meter.MeterProvider<Counter> counterMeterProvider = Counter.builder("jmh.existing").withRegistry(registry);
5151

52-
Tags tags = Tags.of("key", "value");
52+
Tags tags = Tags.of("k1", "v1");
5353

5454
@Setup
5555
public void setup() {
@@ -89,8 +89,8 @@ public Meter registerStale() {
8989
}
9090

9191
@Benchmark
92-
public Meter registerExisting() {
93-
return registry.counter("jmh.existing", "k1", "v1");
92+
public Meter registerExistingCounter() {
93+
return registry.counter("jmh.existing", tags);
9494
}
9595

9696
@Benchmark

implementations/micrometer-registry-dynatrace/src/test/java/io/micrometer/dynatrace/types/DynatraceTimerTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class DynatraceTimerTest {
4747

4848
private static final DistributionStatisticConfig DISTRIBUTION_STATISTIC_CONFIG = DistributionStatisticConfig.NONE;
4949

50-
private static final PauseDetector PAUSE_DETECTOR = new NoPauseDetector();
50+
private static final PauseDetector PAUSE_DETECTOR = NoPauseDetector.INSTANCE;
5151

5252
@Test
5353
void testTimerCount() {

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

Lines changed: 108 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
import io.micrometer.core.instrument.search.RequiredSearch;
3232
import io.micrometer.core.instrument.search.Search;
3333
import io.micrometer.core.instrument.util.TimeUtils;
34+
import org.jspecify.annotations.NonNull;
35+
import org.jspecify.annotations.NullUnmarked;
3436
import org.jspecify.annotations.Nullable;
3537

3638
import java.time.Duration;
@@ -106,7 +108,7 @@ public abstract class MeterRegistry {
106108

107109
/**
108110
* write/remove guarded by meterMapLock, read in
109-
* {@link #getOrCreateMeter(DistributionStatisticConfig, BiFunction, Meter.Id, Function)}
111+
* {@link #getOrCreateMeter(DistributionStatisticConfig, PauseDetector, NewMeterSupplier, Meter.Id, Function)}
110112
* is unguarded
111113
*/
112114
private final Map<Meter.Id, Meter> preFilterIdToMeterMap = new HashMap<>();
@@ -135,7 +137,7 @@ public abstract class MeterRegistry {
135137

136138
private final AtomicBoolean closed = new AtomicBoolean();
137139

138-
private PauseDetector pauseDetector = new NoPauseDetector();
140+
private PauseDetector pauseDetector = NoPauseDetector.INSTANCE;
139141

140142
private @Nullable HighCardinalityTagsDetector highCardinalityTagsDetector;
141143

@@ -335,12 +337,15 @@ private String getBaseTimeUnitStr() {
335337
}
336338

337339
/**
338-
* Only used by {@link Counter#builder(String)}.
340+
* Only used internally.
339341
* @param id The identifier for this counter.
340342
* @return A new or existing counter.
343+
* @implNote Avoids allocation in the case the meter is already registered,
344+
* particularly capturing lambdas.
341345
*/
342346
Counter counter(Meter.Id id) {
343-
return registerMeterIfNecessary(Counter.class, id, this::newCounter, NoopCounter::new);
347+
return registerMeterIfNecessary(Counter.class, id, null, null,
348+
(registry, mappedId, mappedConfig, pd) -> registry.newCounter(mappedId), NoopCounter::new);
344349
}
345350

346351
/**
@@ -352,22 +357,27 @@ Counter counter(Meter.Id id) {
352357
* @return A new or existing gauge.
353358
*/
354359
<T> Gauge gauge(Meter.Id id, @Nullable T obj, ToDoubleFunction<T> valueFunction) {
355-
return registerMeterIfNecessary(Gauge.class, id, id2 -> newGauge(id2, obj, valueFunction), NoopGauge::new);
360+
return registerMeterIfNecessary(Gauge.class, id,
361+
(registry, mappedId) -> registry.newGauge(mappedId, obj, valueFunction), NoopGauge::new);
356362
}
357363

358364
/**
359-
* Only used by {@link Timer#builder(String)}.
365+
* Only used internally.
360366
* @param id The identifier for this timer.
361367
* @param distributionStatisticConfig Configuration that governs how distribution
362368
* statistics are computed.
369+
* @param specificPauseDetector explicit pause detector to use that may be different
370+
* from the registry-configured one
363371
* @return A new or existing timer.
372+
* @implNote Avoids allocation in the case the meter is already registered,
373+
* particularly capturing lambdas.
364374
*/
365375
Timer timer(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig,
366-
PauseDetector pauseDetectorOverride) {
367-
return registerMeterIfNecessary(Timer.class, id, distributionStatisticConfig, (id2, filteredConfig) -> {
368-
Meter.Id withUnit = id2.withBaseUnit(getBaseTimeUnitStr());
369-
return newTimer(withUnit, filteredConfig.merge(defaultHistogramConfig()), pauseDetectorOverride);
370-
}, NoopTimer::new);
376+
PauseDetector specificPauseDetector) {
377+
return registerMeterIfNecessary(Timer.class, id, distributionStatisticConfig, specificPauseDetector,
378+
(registry, mappedId, mappedConfig, pd) -> registry
379+
.newTimer(mappedId.withBaseUnit(registry.getBaseTimeUnitStr()), mappedConfig, pd),
380+
NoopTimer::new);
371381
}
372382

373383
/**
@@ -378,8 +388,9 @@ Timer timer(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig
378388
* @return A new or existing distribution summary.
379389
*/
380390
DistributionSummary summary(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig, double scale) {
381-
return registerMeterIfNecessary(DistributionSummary.class, id, distributionStatisticConfig, (id2,
382-
filteredConfig) -> newDistributionSummary(id2, filteredConfig.merge(defaultHistogramConfig()), scale),
391+
return registerMeterIfNecessary(
392+
DistributionSummary.class, id, distributionStatisticConfig, null, (registry, mappedId, filteredConfig,
393+
pd) -> registry.newDistributionSummary(mappedId, filteredConfig, scale),
383394
NoopDistributionSummary::new);
384395
}
385396

@@ -392,7 +403,8 @@ DistributionSummary summary(Meter.Id id, DistributionStatisticConfig distributio
392403
* @return The meter.
393404
*/
394405
Meter register(Meter.Id id, Meter.Type type, Iterable<Measurement> measurements) {
395-
return registerMeterIfNecessary(Meter.class, id, id2 -> newMeter(id2, type, measurements), NoopMeter::new);
406+
return registerMeterIfNecessary(Meter.class, id,
407+
(registry, mappedId) -> registry.newMeter(mappedId, type, measurements), NoopMeter::new);
396408
}
397409

398410
/**
@@ -444,7 +456,7 @@ public RequiredSearch get(String name) {
444456
* @return A new or existing counter.
445457
*/
446458
public Counter counter(String name, Iterable<Tag> tags) {
447-
return Counter.builder(name).tags(tags).register(this);
459+
return counter(name, Tags.of(tags));
448460
}
449461

450462
/**
@@ -458,6 +470,17 @@ public Counter counter(String name, String... tags) {
458470
return counter(name, Tags.of(tags));
459471
}
460472

473+
/**
474+
* Tracks a monotonically increasing value.
475+
* @param name The base metric name
476+
* @param tags Sequence of dimensions for breaking down the name.
477+
* @return A new or existing counter.
478+
* @since 1.16.0
479+
*/
480+
public Counter counter(String name, Tags tags) {
481+
return counter(new Meter.Id(name, tags, null, null, Meter.Type.COUNTER));
482+
}
483+
461484
/**
462485
* Measures the distribution of samples.
463486
* @param name The base metric name
@@ -486,8 +509,7 @@ public DistributionSummary summary(String name, String... tags) {
486509
* @return A new or existing timer.
487510
*/
488511
public Timer timer(String name, Iterable<Tag> tags) {
489-
return this.timer(new Meter.Id(name, Tags.of(tags), null, null, Meter.Type.TIMER),
490-
AbstractTimerBuilder.DEFAULT_DISTRIBUTION_CONFIG, pauseDetector);
512+
return this.timer(name, Tags.of(tags));
491513
}
492514

493515
/**
@@ -501,6 +523,18 @@ public Timer timer(String name, String... tags) {
501523
return timer(name, Tags.of(tags));
502524
}
503525

526+
/**
527+
* Measures the time taken for short tasks and the count of these tasks.
528+
* @param name The base metric name
529+
* @param tags Sequence of dimensions for breaking down the name.
530+
* @return A new or existing timer.
531+
* @since 1.16.0
532+
*/
533+
public Timer timer(String name, Tags tags) {
534+
return this.timer(new Meter.Id(name, tags, null, null, Meter.Type.TIMER),
535+
AbstractTimerBuilder.DEFAULT_DISTRIBUTION_CONFIG, pauseDetector);
536+
}
537+
504538
/**
505539
* Access to less frequently used meter types and patterns.
506540
* @return Access to additional meter types and patterns.
@@ -607,14 +641,15 @@ public <T extends Collection<?>> T gaugeCollectionSize(String name, Iterable<Tag
607641
}
608642

609643
private <M extends Meter> M registerMeterIfNecessary(Class<M> meterClass, Meter.Id id,
610-
Function<Meter.Id, M> builder, Function<Meter.Id, M> noopBuilder) {
611-
return registerMeterIfNecessary(meterClass, id, null, (id2, conf) -> builder.apply(id2), noopBuilder);
644+
BiFunction<MeterRegistry, Meter.Id, M> meterSupplier, Function<Meter.Id, M> noopBuilder) {
645+
return registerMeterIfNecessary(meterClass, id, null, null,
646+
(registry, mappedId, conf, pd) -> meterSupplier.apply(registry, mappedId), noopBuilder);
612647
}
613648

614649
private <M extends Meter> M registerMeterIfNecessary(Class<M> meterClass, Meter.Id id,
615-
@Nullable DistributionStatisticConfig config, BiFunction<Meter.Id, DistributionStatisticConfig, M> builder,
616-
Function<Meter.Id, M> noopBuilder) {
617-
Meter m = getOrCreateMeter(config, builder, id, noopBuilder);
650+
@Nullable DistributionStatisticConfig config, @Nullable PauseDetector specificPauseDetector,
651+
NewMeterSupplier<M> meterSupplier, Function<Meter.Id, M> noopBuilder) {
652+
Meter m = getOrCreateMeter(config, specificPauseDetector, meterSupplier, id, noopBuilder);
618653

619654
if (!meterClass.isInstance(m)) {
620655
throw new IllegalArgumentException(
@@ -644,7 +679,7 @@ private Meter.Id mapId(Meter.Id id) {
644679
}
645680

646681
private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
647-
BiFunction<Meter.Id, /* Nullable Generic */ DistributionStatisticConfig, ? extends Meter> builder,
682+
@Nullable PauseDetector specificPauseDetector, NewMeterSupplier<? extends Meter> meterSupplier,
648683
Meter.Id originalId, Function<Meter.Id, ? extends Meter> noopBuilder) {
649684

650685
Meter m = preFilterIdToMeterMap.get(originalId);
@@ -684,9 +719,10 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
684719
config = filteredConfig;
685720
}
686721
}
722+
config = config.merge(defaultHistogramConfig());
687723
}
688724

689-
m = builder.apply(mappedId, config);
725+
m = meterSupplier.create(this, mappedId, config, specificPauseDetector);
690726

691727
Meter.Id synAssoc = mappedId.syntheticAssociation();
692728
if (synAssoc != null) {
@@ -1058,21 +1094,33 @@ public LongTaskTimer longTaskTimer(String name, String... tags) {
10581094
* @return A new or existing long task timer.
10591095
*/
10601096
public LongTaskTimer longTaskTimer(String name, Iterable<Tag> tags) {
1061-
return longTaskTimer(new Meter.Id(name, Tags.of(tags), null, null, Meter.Type.LONG_TASK_TIMER),
1097+
return longTaskTimer(name, Tags.of(tags));
1098+
}
1099+
1100+
/**
1101+
* Measures the time taken for long tasks.
1102+
* @param name Name of the long task timer being registered.
1103+
* @param tags Sequence of dimensions for breaking down the name.
1104+
* @return A new or existing long task timer.
1105+
* @since 1.16.0
1106+
*/
1107+
public LongTaskTimer longTaskTimer(String name, Tags tags) {
1108+
return longTaskTimer(new Meter.Id(name, tags, null, null, Meter.Type.LONG_TASK_TIMER),
10621109
LongTaskTimer.Builder.DEFAULT_DISTRIBUTION_CONFIG);
10631110
}
10641111

10651112
/**
1066-
* Only used by {@link LongTaskTimer#builder(String)}.
1113+
* Only used internally.
10671114
* @param id The identifier for this long task timer.
10681115
* @return A new or existing long task timer.
1116+
* @implNote Avoids allocation in the case the meter is already registered,
1117+
* particularly capturing lambdas.
10691118
*/
10701119
LongTaskTimer longTaskTimer(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig) {
1071-
return registerMeterIfNecessary(LongTaskTimer.class, id, distributionStatisticConfig,
1072-
(id2, filteredConfig) -> {
1073-
Meter.Id withUnit = id2.withBaseUnit(getBaseTimeUnitStr());
1074-
return newLongTaskTimer(withUnit, filteredConfig.merge(defaultHistogramConfig()));
1075-
}, NoopLongTaskTimer::new);
1120+
return registerMeterIfNecessary(LongTaskTimer.class, id, distributionStatisticConfig, null,
1121+
(registry, mappedId, mappedConfig, pd) -> registry
1122+
.newLongTaskTimer(mappedId.withBaseUnit(registry.getBaseTimeUnitStr()), mappedConfig),
1123+
NoopLongTaskTimer::new);
10761124
}
10771125

10781126
/**
@@ -1116,7 +1164,8 @@ public <T extends Number> FunctionCounter counter(String name, Iterable<Tag> tag
11161164
*/
11171165
<T> FunctionCounter counter(Meter.Id id, T obj, ToDoubleFunction<T> countFunction) {
11181166
return registerMeterIfNecessary(FunctionCounter.class, id,
1119-
id2 -> newFunctionCounter(id2, obj, countFunction), NoopFunctionCounter::new);
1167+
(registry, mappedId) -> registry.newFunctionCounter(mappedId, obj, countFunction),
1168+
NoopFunctionCounter::new);
11201169
}
11211170

11221171
/**
@@ -1157,10 +1206,10 @@ public <T> FunctionTimer timer(String name, Iterable<Tag> tags, T obj, ToLongFun
11571206
*/
11581207
<T> FunctionTimer timer(Meter.Id id, T obj, ToLongFunction<T> countFunction,
11591208
ToDoubleFunction<T> totalTimeFunction, TimeUnit totalTimeFunctionUnit) {
1160-
return registerMeterIfNecessary(FunctionTimer.class, id, id2 -> {
1161-
Meter.Id withUnit = id2.withBaseUnit(getBaseTimeUnitStr());
1162-
return newFunctionTimer(withUnit, obj, countFunction, totalTimeFunction, totalTimeFunctionUnit);
1163-
}, NoopFunctionTimer::new);
1209+
return registerMeterIfNecessary(FunctionTimer.class, id,
1210+
(registry, mappedId) -> registry.newFunctionTimer(mappedId.withBaseUnit(getBaseTimeUnitStr()), obj,
1211+
countFunction, totalTimeFunction, totalTimeFunctionUnit),
1212+
NoopFunctionTimer::new);
11641213
}
11651214

11661215
/**
@@ -1198,7 +1247,8 @@ public <T> TimeGauge timeGauge(String name, Iterable<Tag> tags, T obj, TimeUnit
11981247
<T> TimeGauge timeGauge(Meter.Id id, @Nullable T obj, TimeUnit timeFunctionUnit,
11991248
ToDoubleFunction<T> timeFunction) {
12001249
return registerMeterIfNecessary(TimeGauge.class, id,
1201-
id2 -> newTimeGauge(id2, obj, timeFunctionUnit, timeFunction), NoopTimeGauge::new);
1250+
(registry, mappedId) -> registry.newTimeGauge(mappedId, obj, timeFunctionUnit, timeFunction),
1251+
NoopTimeGauge::new);
12021252
}
12031253

12041254
}
@@ -1242,6 +1292,27 @@ protected void meterRegistrationFailed(Meter.Id id, @Nullable String reason) {
12421292
}
12431293
}
12441294

1295+
@FunctionalInterface
1296+
// Brittle, but this is internal. We pass nulls for some meter types as explained in
1297+
// JavaDoc.
1298+
@NullUnmarked
1299+
private interface NewMeterSupplier<M extends Meter> {
1300+
1301+
/**
1302+
* Create a new meter with the given parameters. The DistributionStatisticConfig
1303+
* and PauseDetector will be null for meter types that do not take them.
1304+
* @param registry the registry from which to make the new meter
1305+
* @param id the ID of the meter to create
1306+
* @param distributionStatisticConfig optional distribution config for types that
1307+
* take it
1308+
* @param pauseDetector optional pause detector for types that take it
1309+
* @return a new meter
1310+
*/
1311+
@NonNull M create(MeterRegistry registry, Meter.Id id, DistributionStatisticConfig distributionStatisticConfig,
1312+
PauseDetector pauseDetector);
1313+
1314+
}
1315+
12451316
// VisibleForTesting
12461317
Map<Meter.Id, Meter> _getPreFilterIdToMeterMap() {
12471318
return Collections.unmodifiableMap(preFilterIdToMeterMap);

micrometer-core/src/main/java/io/micrometer/core/instrument/distribution/pause/NoPauseDetector.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,22 @@
1515
*/
1616
package io.micrometer.core.instrument.distribution.pause;
1717

18+
/**
19+
* No-op implementation of a {@link PauseDetector}.
20+
*/
1821
public class NoPauseDetector implements PauseDetector {
1922

23+
/**
24+
* Singleton instance of {@link NoPauseDetector}.
25+
* @since 1.16.0
26+
*/
27+
public static final NoPauseDetector INSTANCE = new NoPauseDetector();
28+
29+
/**
30+
* @deprecated use {@link #INSTANCE} instead.
31+
*/
32+
@Deprecated
33+
public NoPauseDetector() {
34+
}
35+
2036
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ static class MyTimer extends AbstractTimer {
7575

7676
MyTimer() {
7777
super(new Meter.Id("name", Tags.empty(), null, null, Meter.Type.TIMER), Clock.SYSTEM,
78-
DistributionStatisticConfig.DEFAULT, new NoPauseDetector(), TimeUnit.SECONDS, false);
78+
DistributionStatisticConfig.DEFAULT, NoPauseDetector.INSTANCE, TimeUnit.SECONDS, false);
7979
}
8080

8181
@Override

0 commit comments

Comments
 (0)