Avoid potential allocation from iterator empty check in Tags/KeyValues

benchmarks/benchmarks-core/build.gradle

@@ -12,9 +12,9 @@ plugins {
 
 dependencies {
     jmh project(':micrometer-core')
-//    jmh 'io.micrometer:micrometer-core:1.13.0-M2'
+//    jmh 'io.micrometer:micrometer-core:1.13.14'
     jmh project(':micrometer-registry-prometheus')
-//    jmh 'io.micrometer:micrometer-registry-prometheus:1.13.0-M2'
+//    jmh 'io.micrometer:micrometer-registry-prometheus:1.13.14'
 
     jmh libs.dropwizardMetricsCore5
     jmh libs.prometheusMetrics

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

@@ -15,35 +15,128 @@
  */
 package io.micrometer.benchmark.core;
 
+import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Tags;
 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.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.concurrent.TimeUnit;
 
-@Fork(1)
-@Measurement(iterations = 2)
-@Warmup(iterations = 2)
-@BenchmarkMode(Mode.AverageTime)
-@OutputTimeUnit(TimeUnit.NANOSECONDS)
 public class TagsBenchmark {
 
-    @Benchmark
-    public Tags of() {
-        return Tags.of("key", "value", "key2", "value2", "key3", "value3", "key4", "value4", "key5", "value5");
-    }
+    @Fork(1)
+    @Measurement(iterations = 2)
+    @Warmup(iterations = 2)
+    @BenchmarkMode(Mode.AverageTime)
+    @OutputTimeUnit(TimeUnit.NANOSECONDS)
+    public static class TagsOfBenchmark {
+
+        @Benchmark
+        public Tags ofStringVarargs() {
+            return Tags.of("key", "value", "key2", "value2", "key3", "value3", "key4", "value4", "key5", "value5");
+        }
+
+        @Benchmark
+        public Tags ofTagVarargs() {
+            return Tags.of(Tag.of("key", "value"), Tag.of("key2", "value2"), Tag.of("key3", "value3"),
+                    Tag.of("key4", "value4"), Tag.of("key5", "value5"));
+        }
+
+        @Benchmark
+        public Tags ofTags() {
+            return Tags
+                .of(Tags.of("key", "value", "key2", "value2", "key3", "value3", "key4", "value4", "key5", "value5"));
+        }
+
+        @Benchmark
+        public Tags ofArrayList() {
+            List<Tag> tags = new ArrayList<>(5);
+            tags.add(Tag.of("key", "value"));
+            tags.add(Tag.of("key2", "value2"));
+            tags.add(Tag.of("key3", "value3"));
+            tags.add(Tag.of("key4", "value4"));
+            tags.add(Tag.of("key5", "value5"));
+            return Tags.of(tags);
+        }
+
+        @Benchmark
+        public Tags ofEmptyCollection() {
+            return Tags.of(Collections.emptyList());
+        }
+
+        @Benchmark
+        public Tags ofEmptyTags() {
+            return Tags.of(Tags.empty());
+        }
+
+        public static void main(String[] args) throws RunnerException {
+            Options opt = new OptionsBuilder().include(TagsOfBenchmark.class.getSimpleName())
+                .addProfiler(GCProfiler.class)
+                .build();
+            new Runner(opt).run();
+        }
 
-    @Benchmark
-    public Tags dotAnd() {
-        return Tags.of("key", "value").and("key2", "value2", "key3", "value3", "key4", "value4", "key5", "value5");
     }
 
-    public static void main(String[] args) throws RunnerException {
-        Options opt = new OptionsBuilder().include(TagsBenchmark.class.getSimpleName()).build();
-        new Runner(opt).run();
+    @Fork(1)
+    @Measurement(iterations = 3)
+    @Warmup(iterations = 5)
+    @BenchmarkMode(Mode.AverageTime)
+    @OutputTimeUnit(TimeUnit.NANOSECONDS)
+    public static class TagsAndBenchmark {
+
+        @Benchmark
+        public Tags andVarargsString() { // allocating more; 360 B/op vs 336 on 1.13.14
+            return Tags.of("key", "value").and("key2", "value2", "key3", "value3", "key4", "value4", "key5", "value5");
+        }
+
+        @Benchmark
+        public Tags andTagVarargs() {
+            return Tags.of("key", "value")
+                .and(Tag.of("key2", "value2"), Tag.of("key3", "value3"), Tag.of("key4", "value4"),
+                        Tag.of("key5", "value5"));
+        }
+
+        @Benchmark
+        public Tags andTags() { // allocating more; 360 B/op vs 336 on 1.13.14
+            return Tags.of("key", "value")
+                .and(Tags.of("key2", "value2", "key3", "value3", "key4", "value4", "key5", "value5"));
+        }
+
+        @Benchmark
+        public Tags andArrayList() {
+            List<Tag> tags = new ArrayList<>(4);
+            tags.add(Tag.of("key2", "value2"));
+            tags.add(Tag.of("key3", "value3"));
+            tags.add(Tag.of("key4", "value4"));
+            tags.add(Tag.of("key5", "value5"));
+            return Tags.of("key", "value").and(tags);
+        }
+
+        @Benchmark
+        public Tags andEmptyCollection() {
+            return Tags.of("key", "value").and(Collections.emptyList());
+        }
+
+        @Benchmark
+        public Tags andEmptyTags() {
+            return Tags.of("key", "value").and(Tags.empty());
+        }
+
+        public static void main(String[] args) throws RunnerException {
+            Options opt = new OptionsBuilder().include(TagsAndBenchmark.class.getSimpleName())
+                .addProfiler(GCProfiler.class)
+                .build();
+            new Runner(opt).run();
+        }
+
     }
 
 }

micrometer-commons/src/main/java/io/micrometer/common/KeyValues.java

@@ -133,13 +133,15 @@ public <E> KeyValues and(@Nullable Iterable<E> elements, Function<E, String> key
      * @return a new {@code KeyValues} instance
      */
     public KeyValues and(@Nullable Iterable<? extends KeyValue> keyValues) {
-        if (keyValues == null || keyValues == EMPTY || !keyValues.iterator().hasNext()) {
+        if (keyValues == null || keyValues == EMPTY) {
             return this;
         }
-
-        if (this.keyValues.length == 0) {
+        else if (this.keyValues.length == 0) {
             return KeyValues.of(keyValues);
         }
+        else if (!keyValues.iterator().hasNext()) {
+            return this;
+        }
 
         return and(KeyValues.of(keyValues).keyValues);
     }
@@ -258,12 +260,15 @@ public static <E> KeyValues of(@Nullable Iterable<E> elements, Function<E, Strin
      * @return a new {@code KeyValues} instance
      */
     public static KeyValues of(@Nullable Iterable<? extends KeyValue> keyValues) {
-        if (keyValues == null || keyValues == EMPTY || !keyValues.iterator().hasNext()) {
+        if (keyValues == null || keyValues == EMPTY) {
             return KeyValues.empty();
         }
         else if (keyValues instanceof KeyValues) {
             return (KeyValues) keyValues;
         }
+        else if (!keyValues.iterator().hasNext()) {
+            return KeyValues.empty();
+        }
         else if (keyValues instanceof Collection) {
             Collection<? extends KeyValue> keyValuesCollection = (Collection<? extends KeyValue>) keyValues;
             return new KeyValues(keyValuesCollection.toArray(new KeyValue[0]));

micrometer-commons/src/test/java/io/micrometer/common/AllocationTest.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2025 the original author or authors.
+ *
+ * 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.common;
+
+import org.junit.jupiter.api.condition.DisabledIfSystemProperty;
+import org.junit.jupiter.params.ParameterizedTest;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@Retention(RetentionPolicy.RUNTIME)
+@ParameterizedTest
+@DisabledIfSystemProperty(named = "java.vm.name", matches = AllocationTest.JAVA_VM_NAME_J9_REGEX,
+        disabledReason = "Sun ThreadMXBean with allocation counter not available")
+public @interface AllocationTest {
+
+    // Should match "Eclipse OpenJ9 VM" and "IBM J9 VM"
+    String JAVA_VM_NAME_J9_REGEX = ".*J9 VM$";
+
+}

micrometer-commons/src/test/java/io/micrometer/common/KeyValuesTest.java

@@ -18,9 +18,8 @@
 
 import com.sun.management.ThreadMXBean;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.condition.DisabledForJreRange;
-import org.junit.jupiter.api.condition.DisabledIfSystemProperty;
-import org.junit.jupiter.api.condition.JRE;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import java.lang.management.ManagementFactory;
 import java.util.*;
@@ -39,9 +38,6 @@
  */
 class KeyValuesTest {
 
-    // Should match "Eclipse OpenJ9 VM" and "IBM J9 VM"
-    private static final String JAVA_VM_NAME_J9_REGEX = ".*J9 VM$";
-
     @Test
     void dedup() {
         assertThat(KeyValues.of("k1", "v1", "k2", "v2")).containsExactly(KeyValue.of("k1", "v1"),
@@ -340,42 +336,56 @@ void emptyShouldNotContainKeyValues() {
     }
 
     // gh-3313
-    @Test
-    @DisabledIfSystemProperty(named = "java.vm.name", matches = JAVA_VM_NAME_J9_REGEX,
-            disabledReason = "Sun ThreadMXBean with allocation counter not available")
-    @DisabledForJreRange(min = JRE.JAVA_19, max = JRE.JAVA_19,
-            disabledReason = "https://github.com/micrometer-metrics/micrometer/issues/3436")
-    void andEmptyDoesNotAllocate() {
-        ThreadMXBean threadMXBean = (ThreadMXBean) ManagementFactory.getThreadMXBean();
-        long currentThreadId = Thread.currentThread().getId();
+    @AllocationTest
+    @MethodSource("emptyNull_noAllocationArgs")
+    void anyKeyValuesAnd_noAllocation(Iterable<KeyValue> arg) {
         KeyValues keyValues = KeyValues.of("a", "b");
-        KeyValues extraKeyValues = KeyValues.empty();
 
-        long allocatedBytesBefore = threadMXBean.getThreadAllocatedBytes(currentThreadId);
-        KeyValues combined = keyValues.and(extraKeyValues);
-        long allocatedBytes = threadMXBean.getThreadAllocatedBytes(currentThreadId) - allocatedBytesBefore;
+        assertNoAllocations(() -> keyValues.and(arg));
+    }
 
-        assertThat(combined).isEqualTo(keyValues);
-        assertThat(allocatedBytes).isEqualTo(0);
+    @ParameterizedTest
+    @MethodSource("emptyNull_noAllocationArgs")
+    void anyKeyValuesAnd_sameAsThis(Iterable<KeyValue> arg) {
+        KeyValues tags = KeyValues.of("a", "b");
+        KeyValues combined = tags.and(arg);
+
+        assertThat(combined).isSameAs(tags);
+    }
+
+    static Stream<Iterable<KeyValue>> emptyNull_noAllocationArgs() {
+        // Note, new ArrayList<>() etc will allocate an iterator
+        return Stream.of(KeyValues.empty(), Collections.emptyList(), null);
+    }
+
+    @AllocationTest
+    @MethodSource("nonEmptyKeyValues_noAllocationArgs")
+    @MethodSource("emptyNull_noAllocationArgs")
+    void emptyAnd_noAllocation(Iterable<KeyValue> arg) {
+        assertNoAllocations(() -> KeyValues.empty().and(arg));
     }
 
     // gh-3313
-    @Test
-    @DisabledIfSystemProperty(named = "java.vm.name", matches = JAVA_VM_NAME_J9_REGEX,
-            disabledReason = "Sun ThreadMXBean with allocation counter not available")
-    @DisabledForJreRange(min = JRE.JAVA_19, max = JRE.JAVA_19,
-            disabledReason = "https://github.com/micrometer-metrics/micrometer/issues/3436")
-    void ofEmptyDoesNotAllocate() {
+    @AllocationTest
+    @MethodSource("nonEmptyKeyValues_noAllocationArgs")
+    @MethodSource("emptyNull_noAllocationArgs")
+    void of_noAllocation(Iterable<KeyValue> arg) {
+        assertNoAllocations(() -> KeyValues.of(arg));
+    }
+
+    static Stream<Iterable<KeyValue>> nonEmptyKeyValues_noAllocationArgs() {
+        return Stream.of(KeyValues.of("any", "thing"));
+    }
+
+    private void assertNoAllocations(Runnable runnable) {
         ThreadMXBean threadMXBean = (ThreadMXBean) ManagementFactory.getThreadMXBean();
         long currentThreadId = Thread.currentThread().getId();
-        KeyValues extraKeyValues = KeyValues.empty();
 
         long allocatedBytesBefore = threadMXBean.getThreadAllocatedBytes(currentThreadId);
-        KeyValues of = KeyValues.of(extraKeyValues);
+        runnable.run();
         long allocatedBytes = threadMXBean.getThreadAllocatedBytes(currentThreadId) - allocatedBytesBefore;
 
-        assertThat(of).isEqualTo(KeyValues.empty());
-        assertThat(allocatedBytes).isEqualTo(0);
+        assertThat(allocatedBytes).isZero();
     }
 
     private void assertKeyValues(KeyValues keyValues, String... expectedKeyValues) {

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

@@ -112,13 +112,15 @@ public Tags and(@Nullable Tag... tags) {
      * @return a new {@code Tags} instance
      */
     public Tags and(@Nullable Iterable<? extends Tag> tags) {
-        if (tags == null || tags == EMPTY || !tags.iterator().hasNext()) {
+        if (tags == null || tags == EMPTY) {
             return this;
         }
-
-        if (this.tags.length == 0) {
+        else if (this.tags.length == 0) {
             return Tags.of(tags);
         }
+        else if (!tags.iterator().hasNext()) {
+            return this;
+        }
 
         return and(Tags.of(tags).tags);
     }
@@ -221,12 +223,15 @@ public static Tags concat(@Nullable Iterable<? extends Tag> tags, @Nullable Stri
      * @return a new {@code Tags} instance
      */
     public static Tags of(@Nullable Iterable<? extends Tag> tags) {
-        if (tags == null || tags == EMPTY || !tags.iterator().hasNext()) {
+        if (tags == null || tags == EMPTY) {
             return Tags.empty();
         }
         else if (tags instanceof Tags) {
             return (Tags) tags;
         }
+        else if (!tags.iterator().hasNext()) {
+            return Tags.empty();
+        }
         else if (tags instanceof Collection) {
             Collection<? extends Tag> tagsCollection = (Collection<? extends Tag>) tags;
             return new Tags(tagsCollection.toArray(new Tag[0]));

micrometer-core/src/test/java/io/micrometer/core/AllocationTest.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2025 the original author or authors.
+ *
+ * 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.core;
+
+import org.junit.jupiter.api.condition.DisabledIfSystemProperty;
+import org.junit.jupiter.params.ParameterizedTest;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@Retention(RetentionPolicy.RUNTIME)
+@ParameterizedTest
+@DisabledIfSystemProperty(named = "java.vm.name", matches = AllocationTest.JAVA_VM_NAME_J9_REGEX,
+        disabledReason = "Sun ThreadMXBean with allocation counter not available")
+public @interface AllocationTest {
+
+    // Should match "Eclipse OpenJ9 VM" and "IBM J9 VM"
+    String JAVA_VM_NAME_J9_REGEX = ".*J9 VM$";
+
+}