Skip to content

Avoid capturing lambda allocation when retrieving existing meters #6670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 28, 2025
Merged

Avoid capturing lambda allocation when retrieving existing meters #6670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68327bb
Avoid capturing lambda allocation when retrieving existing meters
shakuzen Aug 27, 2025
b1336c5
Add allocation tests
shakuzen Aug 28, 2025
81d9a82
Polish
shakuzen Aug 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ...benchmarks-core/src/jmh/java/io/micrometer/benchmark/core/MeterRegistrationBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
@Warmup(iterations = 2)
@Measurement(iterations = 5)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Threads(2)
public class MeterRegistrationBenchmark {

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

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

Tags tags = Tags.of("key", "value");
Tags tags = Tags.of("k1", "v1");

@Setup
public void setup() {
Expand Down Expand Up @@ -89,8 +89,8 @@ public Meter registerStale() {
}

@Benchmark
public Meter registerExisting() {
return registry.counter("jmh.existing", "k1", "v1");
public Meter registerExistingCounter() {
return registry.counter("jmh.existing", tags);
}

@Benchmark
Expand Down
2 changes: 1 addition & 1 deletion ...er-registry-dynatrace/src/test/java/io/micrometer/dynatrace/types/DynatraceTimerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class DynatraceTimerTest {

private static final DistributionStatisticConfig DISTRIBUTION_STATISTIC_CONFIG = DistributionStatisticConfig.NONE;

private static final PauseDetector PAUSE_DETECTOR = new NoPauseDetector();
private static final PauseDetector PAUSE_DETECTOR = NoPauseDetector.INSTANCE;

@Test
void testTimerCount() {
Expand Down
145 changes: 108 additions & 37 deletions micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import io.micrometer.core.instrument.search.RequiredSearch;
import io.micrometer.core.instrument.search.Search;
import io.micrometer.core.instrument.util.TimeUtils;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.NullUnmarked;
import org.jspecify.annotations.Nullable;

import java.time.Duration;
Expand Down Expand Up @@ -106,7 +108,7 @@ public abstract class MeterRegistry {

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

private final AtomicBoolean closed = new AtomicBoolean();

private PauseDetector pauseDetector = new NoPauseDetector();
private PauseDetector pauseDetector = NoPauseDetector.INSTANCE;

private @Nullable HighCardinalityTagsDetector highCardinalityTagsDetector;

Expand Down Expand Up @@ -335,12 +337,15 @@ private String getBaseTimeUnitStr() {
}

/**
* Only used by {@link Counter#builder(String)}.
* Only used internally.
* @param id The identifier for this counter.
* @return A new or existing counter.
* @implNote Avoids allocation in the case the meter is already registered,
* particularly capturing lambdas.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added these implementation notes to the JavaDoc in a few places. It's quite easy to not notice that some code written is a capturing lambda that will likely cause allocation. I suspect we were not consciously aware of that for this code before (I don't recall being aware of that in this part of the code).

We should come up with a way to make sure this doesn't regress. We have some tests that assert there were no allocations in some executed code. Perhaps we could do similar with these. Asserting no allocations is generally easier than asserting a specific number of bytes allocated, since that depends on the JVM and environment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added tests for this in a subsequent commit. From the non-public methods asserting no allocations worked fine, but to assert only the ID allocation from a public method required adding public API variants of existing API that takes a Tags instance to avoid calling Tags.of which may allocate before JIT optimizations.

*/
Counter counter(Meter.Id id) {
return registerMeterIfNecessary(Counter.class, id, this::newCounter, NoopCounter::new);
return registerMeterIfNecessary(Counter.class, id, null, null,
(registry, mappedId, mappedConfig, pd) -> registry.newCounter(mappedId), NoopCounter::new);
}

/**
Expand All @@ -352,22 +357,27 @@ Counter counter(Meter.Id id) {
* @return A new or existing gauge.
*/
<T> Gauge gauge(Meter.Id id, @Nullable T obj, ToDoubleFunction<T> valueFunction) {
return registerMeterIfNecessary(Gauge.class, id, id2 -> newGauge(id2, obj, valueFunction), NoopGauge::new);
return registerMeterIfNecessary(Gauge.class, id,
(registry, mappedId) -> registry.newGauge(mappedId, obj, valueFunction), NoopGauge::new);
}

/**
* Only used by {@link Timer#builder(String)}.
* Only used internally.
* @param id The identifier for this timer.
* @param distributionStatisticConfig Configuration that governs how distribution
* statistics are computed.
* @param specificPauseDetector explicit pause detector to use that may be different
* from the registry-configured one
* @return A new or existing timer.
* @implNote Avoids allocation in the case the meter is already registered,
* particularly capturing lambdas.
*/
Timer timer(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig,
PauseDetector pauseDetectorOverride) {
return registerMeterIfNecessary(Timer.class, id, distributionStatisticConfig, (id2, filteredConfig) -> {
Meter.Id withUnit = id2.withBaseUnit(getBaseTimeUnitStr());
return newTimer(withUnit, filteredConfig.merge(defaultHistogramConfig()), pauseDetectorOverride);
Copy link
Member Author

@shakuzen shakuzen Aug 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging with the default histogram config was moved into getOrCreateMeter when the passed DistributionStatisticConfig is not null. I think I initially did this in an effort to get rid of references in the lambda body that would be captured. Since things ended up with the registry as a parameter, I'm not sure it matters where the code is; it should be the same either way (famous last words). I suppose it's a little less duplication to have it in getOrCreateMeter, but it makes the method body longer in a critical function, which may make it harder to read the code.

Separately, it occurs to me that the method defaultHistogramConfig() will most likely always return an equivalent instance but it builds and merges it every time in most implementations. This is wasteful and could likely be improved. It only affects new meters, but it's still wasteful if it's always returning an equivalent config. I did at one point try one naive approach which didn't work, so it can be left as future work to be considered.

}, NoopTimer::new);
PauseDetector specificPauseDetector) {
return registerMeterIfNecessary(Timer.class, id, distributionStatisticConfig, specificPauseDetector,
(registry, mappedId, mappedConfig, pd) -> registry
.newTimer(mappedId.withBaseUnit(registry.getBaseTimeUnitStr()), mappedConfig, pd),
NoopTimer::new);
}

/**
Expand All @@ -378,8 +388,9 @@ Timer timer(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig
* @return A new or existing distribution summary.
*/
DistributionSummary summary(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig, double scale) {
return registerMeterIfNecessary(DistributionSummary.class, id, distributionStatisticConfig, (id2,
filteredConfig) -> newDistributionSummary(id2, filteredConfig.merge(defaultHistogramConfig()), scale),
return registerMeterIfNecessary(
DistributionSummary.class, id, distributionStatisticConfig, null, (registry, mappedId, filteredConfig,
pd) -> registry.newDistributionSummary(mappedId, filteredConfig, scale),
NoopDistributionSummary::new);
}

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

/**
Expand Down Expand Up @@ -444,7 +456,7 @@ public RequiredSearch get(String name) {
* @return A new or existing counter.
*/
public Counter counter(String name, Iterable<Tag> tags) {
return Counter.builder(name).tags(tags).register(this);
return counter(name, Tags.of(tags));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally I left this with the builder to prove the point that JIT can eliminate the apparent allocation that was here. While that's still true, this change along with the new counter method that takes a Tags parameter avoids some allocations even before JIT optimizes. This was useful for the added unit test asserting allocated bytes, where JIT won't have optimized the code. It also helps reduce allocation in applications before JIT optimization (e.g. start-up or low traffic applications).

}

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

/**
* Tracks a monotonically increasing value.
* @param name The base metric name
* @param tags Sequence of dimensions for breaking down the name.
* @return A new or existing counter.
* @since 1.16.0
*/
public Counter counter(String name, Tags tags) {
return counter(new Meter.Id(name, tags, null, null, Meter.Type.COUNTER));
}

/**
* Measures the distribution of samples.
* @param name The base metric name
Expand Down Expand Up @@ -486,8 +509,7 @@ public DistributionSummary summary(String name, String... tags) {
* @return A new or existing timer.
*/
public Timer timer(String name, Iterable<Tag> tags) {
return this.timer(new Meter.Id(name, Tags.of(tags), null, null, Meter.Type.TIMER),
AbstractTimerBuilder.DEFAULT_DISTRIBUTION_CONFIG, pauseDetector);
return this.timer(name, Tags.of(tags));
}

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

/**
* Measures the time taken for short tasks and the count of these tasks.
* @param name The base metric name
* @param tags Sequence of dimensions for breaking down the name.
* @return A new or existing timer.
* @since 1.16.0
*/
public Timer timer(String name, Tags tags) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make this added API (and the equivalent counter/ltt API) non-public if we don't want to add API that's mostly sugar which could overwhelm users with options, but it seems harmless enough, and as mentioned in another comment, there are cases it can improve performance before JIT optimization.

return this.timer(new Meter.Id(name, tags, null, null, Meter.Type.TIMER),
AbstractTimerBuilder.DEFAULT_DISTRIBUTION_CONFIG, pauseDetector);
}

/**
* Access to less frequently used meter types and patterns.
* @return Access to additional meter types and patterns.
Expand Down Expand Up @@ -607,14 +641,15 @@ public <T extends Collection<?>> T gaugeCollectionSize(String name, Iterable<Tag
}

private <M extends Meter> M registerMeterIfNecessary(Class<M> meterClass, Meter.Id id,
Function<Meter.Id, M> builder, Function<Meter.Id, M> noopBuilder) {
return registerMeterIfNecessary(meterClass, id, null, (id2, conf) -> builder.apply(id2), noopBuilder);
BiFunction<MeterRegistry, Meter.Id, M> meterSupplier, Function<Meter.Id, M> noopBuilder) {
return registerMeterIfNecessary(meterClass, id, null, null,
(registry, mappedId, conf, pd) -> meterSupplier.apply(registry, mappedId), noopBuilder);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adapting of the BiFunction to NewMeterSupplier is a capturing lambda that allocates (it did before these changes too), which is why we avoid calling this variant of registerMeterIfNecessary from the Counter call stack. It's convenient though, so we still use it for Function meters that should only ever be registered once anyway.

}

private <M extends Meter> M registerMeterIfNecessary(Class<M> meterClass, Meter.Id id,
@Nullable DistributionStatisticConfig config, BiFunction<Meter.Id, DistributionStatisticConfig, M> builder,
Function<Meter.Id, M> noopBuilder) {
Meter m = getOrCreateMeter(config, builder, id, noopBuilder);
@Nullable DistributionStatisticConfig config, @Nullable PauseDetector specificPauseDetector,
NewMeterSupplier<M> meterSupplier, Function<Meter.Id, M> noopBuilder) {
Meter m = getOrCreateMeter(config, specificPauseDetector, meterSupplier, id, noopBuilder);

if (!meterClass.isInstance(m)) {
throw new IllegalArgumentException(
Expand Down Expand Up @@ -644,7 +679,7 @@ private Meter.Id mapId(Meter.Id id) {
}

private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
BiFunction<Meter.Id, /* Nullable Generic */ DistributionStatisticConfig, ? extends Meter> builder,
@Nullable PauseDetector specificPauseDetector, NewMeterSupplier<? extends Meter> meterSupplier,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We take the pause detector as a parameter here so we don't need to capture it in the lambda for the NewMeterSupplier (previously the BiFunction).

Meter.Id originalId, Function<Meter.Id, ? extends Meter> noopBuilder) {

Meter m = preFilterIdToMeterMap.get(originalId);
Expand Down Expand Up @@ -684,9 +719,10 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
config = filteredConfig;
}
}
config = config.merge(defaultHistogramConfig());
}

m = builder.apply(mappedId, config);
m = meterSupplier.create(this, mappedId, config, specificPauseDetector);

Meter.Id synAssoc = mappedId.syntheticAssociation();
if (synAssoc != null) {
Expand Down Expand Up @@ -1058,21 +1094,33 @@ public LongTaskTimer longTaskTimer(String name, String... tags) {
* @return A new or existing long task timer.
*/
public LongTaskTimer longTaskTimer(String name, Iterable<Tag> tags) {
return longTaskTimer(new Meter.Id(name, Tags.of(tags), null, null, Meter.Type.LONG_TASK_TIMER),
return longTaskTimer(name, Tags.of(tags));
}

/**
* Measures the time taken for long tasks.
* @param name Name of the long task timer being registered.
* @param tags Sequence of dimensions for breaking down the name.
* @return A new or existing long task timer.
* @since 1.16.0
*/
public LongTaskTimer longTaskTimer(String name, Tags tags) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added to avoid the Tags.of call when we already have a Tags instance. After JIT warm-up, it should not make any difference, but until then, calls to Tags.of with a Tags instance can allocate an ArrayIterator. This was an issue in the added unit test where JIT will not have eliminated that allocation.

return longTaskTimer(new Meter.Id(name, tags, null, null, Meter.Type.LONG_TASK_TIMER),
LongTaskTimer.Builder.DEFAULT_DISTRIBUTION_CONFIG);
}

/**
* Only used by {@link LongTaskTimer#builder(String)}.
* Only used internally.
* @param id The identifier for this long task timer.
* @return A new or existing long task timer.
* @implNote Avoids allocation in the case the meter is already registered,
* particularly capturing lambdas.
*/
LongTaskTimer longTaskTimer(Meter.Id id, DistributionStatisticConfig distributionStatisticConfig) {
return registerMeterIfNecessary(LongTaskTimer.class, id, distributionStatisticConfig,
(id2, filteredConfig) -> {
Meter.Id withUnit = id2.withBaseUnit(getBaseTimeUnitStr());
return newLongTaskTimer(withUnit, filteredConfig.merge(defaultHistogramConfig()));
}, NoopLongTaskTimer::new);
return registerMeterIfNecessary(LongTaskTimer.class, id, distributionStatisticConfig, null,
(registry, mappedId, mappedConfig, pd) -> registry
.newLongTaskTimer(mappedId.withBaseUnit(registry.getBaseTimeUnitStr()), mappedConfig),
NoopLongTaskTimer::new);
}

/**
Expand Down Expand Up @@ -1116,7 +1164,8 @@ public <T extends Number> FunctionCounter counter(String name, Iterable<Tag> tag
*/
<T> FunctionCounter counter(Meter.Id id, T obj, ToDoubleFunction<T> countFunction) {
return registerMeterIfNecessary(FunctionCounter.class, id,
id2 -> newFunctionCounter(id2, obj, countFunction), NoopFunctionCounter::new);
(registry, mappedId) -> registry.newFunctionCounter(mappedId, obj, countFunction),
NoopFunctionCounter::new);
}

/**
Expand Down Expand Up @@ -1157,10 +1206,10 @@ public <T> FunctionTimer timer(String name, Iterable<Tag> tags, T obj, ToLongFun
*/
<T> FunctionTimer timer(Meter.Id id, T obj, ToLongFunction<T> countFunction,
ToDoubleFunction<T> totalTimeFunction, TimeUnit totalTimeFunctionUnit) {
return registerMeterIfNecessary(FunctionTimer.class, id, id2 -> {
Meter.Id withUnit = id2.withBaseUnit(getBaseTimeUnitStr());
return newFunctionTimer(withUnit, obj, countFunction, totalTimeFunction, totalTimeFunctionUnit);
}, NoopFunctionTimer::new);
return registerMeterIfNecessary(FunctionTimer.class, id,
(registry, mappedId) -> registry.newFunctionTimer(mappedId.withBaseUnit(getBaseTimeUnitStr()), obj,
countFunction, totalTimeFunction, totalTimeFunctionUnit),
NoopFunctionTimer::new);
}

/**
Expand Down Expand Up @@ -1198,7 +1247,8 @@ public <T> TimeGauge timeGauge(String name, Iterable<Tag> tags, T obj, TimeUnit
<T> TimeGauge timeGauge(Meter.Id id, @Nullable T obj, TimeUnit timeFunctionUnit,
ToDoubleFunction<T> timeFunction) {
return registerMeterIfNecessary(TimeGauge.class, id,
id2 -> newTimeGauge(id2, obj, timeFunctionUnit, timeFunction), NoopTimeGauge::new);
(registry, mappedId) -> registry.newTimeGauge(mappedId, obj, timeFunctionUnit, timeFunction),
NoopTimeGauge::new);
}

}
Expand Down Expand Up @@ -1242,6 +1292,27 @@ protected void meterRegistrationFailed(Meter.Id id, @Nullable String reason) {
}
}

@FunctionalInterface
// Brittle, but this is internal. We pass nulls for some meter types as explained in
// JavaDoc.
@NullUnmarked
private interface NewMeterSupplier<M extends Meter> {

/**
* Create a new meter with the given parameters. The DistributionStatisticConfig
* and PauseDetector will be null for meter types that do not take them.
* @param registry the registry from which to make the new meter
* @param id the ID of the meter to create
* @param distributionStatisticConfig optional distribution config for types that
* take it
* @param pauseDetector optional pause detector for types that take it
* @return a new meter
*/
@NonNull M create(MeterRegistry registry, Meter.Id id, DistributionStatisticConfig distributionStatisticConfig,
PauseDetector pauseDetector);

}

// VisibleForTesting
Map<Meter.Id, Meter> _getPreFilterIdToMeterMap() {
return Collections.unmodifiableMap(preFilterIdToMeterMap);
Expand Down
16 changes: 16 additions & 0 deletions ...-core/src/main/java/io/micrometer/core/instrument/distribution/pause/NoPauseDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,22 @@
*/
package io.micrometer.core.instrument.distribution.pause;

/**
* No-op implementation of a {@link PauseDetector}.
*/
public class NoPauseDetector implements PauseDetector {

/**
* Singleton instance of {@link NoPauseDetector}.
* @since 1.16.0
*/
public static final NoPauseDetector INSTANCE = new NoPauseDetector();

/**
* @deprecated use {@link #INSTANCE} instead.
*/
@Deprecated
public NoPauseDetector() {
}

}
2 changes: 1 addition & 1 deletion micrometer-core/src/test/java/io/micrometer/core/instrument/AbstractTimerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static class MyTimer extends AbstractTimer {

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

@Override
Expand Down
Loading