Skip to content

Commit

Permalink
Concurrency and benchmark tests
Browse files Browse the repository at this point in the history
Adds tests to check concurrent behavior and measure performance of registering meters. Updates the implementation of dealing with stale IDs to make it less prone to stale reads.
  • Loading branch information
shakuzen committed Apr 8, 2024
1 parent 0c47cb9 commit 858b9e7
Show file tree
Hide file tree
Showing 11 changed files with 267 additions and 22 deletions.
16 changes: 13 additions & 3 deletions benchmarks/benchmarks-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,29 @@ plugins {
id "me.champeau.jmh" version "0.7.2"
}

// Uncomment as needed for benchmarking against released versions of Micrometer
//repositories {
// mavenCentral()
// maven {
// url "https://repo.spring.io/milestone"
// }
//}

dependencies {
jmh project(':micrometer-core')
// jmh 'io.micrometer:micrometer-core:1.13.0-M2'
jmh project(':micrometer-registry-prometheus')
// jmh 'io.micrometer:micrometer-registry-prometheus:1.13.0-M2'

jmh libs.dropwizardMetricsCore5
jmh libs.prometheusMetrics

jmh 'io.dropwizard.metrics:metrics-core'
jmh 'com.google.guava:guava'
jmh libs.dropwizardMetricsCore
jmh libs.guava

jmh libs.jmhCore

jmh 'ch.qos.logback:logback-classic'
jmh libs.logback12

// Nebula doesn't like having jmhAnnotationProcessor without jmh so we just add it twice.
jmh libs.jmhAnnotationProcessor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,13 @@
*/
package io.micrometer.benchmark.core;

import io.micrometer.core.instrument.Counter;
import io.micrometer.core.instrument.Meter;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.config.MeterFilter;
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;
Expand All @@ -26,37 +30,60 @@
import java.util.concurrent.TimeUnit;

@State(Scope.Benchmark)
@Fork(1)
@Warmup(iterations = 2)
@Measurement(iterations = 5)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
@Threads(2)
public class MeterRegistrationBenchmark {

public static void main(String[] args) throws RunnerException {
Options opt = new OptionsBuilder().include(MeterRegistrationBenchmark.class.getSimpleName())
.warmupIterations(2)
.measurementIterations(5)
.mode(Mode.SampleTime)
.timeUnit(TimeUnit.SECONDS)
.forks(1)
.addProfiler(GCProfiler.class)
.build();

new Runner(opt).run();
}

private int x = 923;
MeterRegistry registry = new SimpleMeterRegistry();

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

@Setup
public void setup() {
registry.config()
.commonTags("application", "abcservice", "az", "xyz", "environment", "production", "random-meta",
"random-meta");
registry.counter("jmh.stale");
registry.config().meterFilter(MeterFilter.acceptNameStartsWith("jmh"));
registry.counter("jmh.existing", "k1", "v1");
}

@Benchmark
@Warmup(iterations = 20)
@Measurement(iterations = 200)
@BenchmarkMode(Mode.SingleShotTime)
public Meter registerNew() {
return registry.counter("jmh.counter", "k1", "v1");
}

@Benchmark
@Warmup(iterations = 20)
@Measurement(iterations = 200)
@BenchmarkMode(Mode.SingleShotTime)
public Meter registerStale() {
return registry.counter("jmh.stale");
}

@Benchmark
public int insert10_000() {
MeterRegistry registry = new SimpleMeterRegistry();
for (int i = 0; i < 10_000; i++) {
registry.counter("my.counter", "k" + i, "v1");
}
return sum();
public Meter registerExisting() {
return registry.counter("jmh.existing", "k1", "v1");
}

@Benchmark
public int sum() {
return x + y;
public Meter registerExistingWithProvider() {
return counterMeterProvider.withTag("k1", "v1");
}

}
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ subprojects {
}

// Do not publish some modules
if (!['samples', 'benchmarks', 'micrometer-osgi-test'].find { project.name.contains(it) }) {
if (!['samples', 'benchmarks', 'micrometer-osgi-test', 'concurrency-tests'].find { project.name.contains(it) }) {
apply plugin: 'com.netflix.nebula.maven-publish'
apply plugin: 'com.netflix.nebula.maven-manifest'
apply plugin: 'com.netflix.nebula.maven-developer'
Expand Down
15 changes: 15 additions & 0 deletions concurrency-tests/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
plugins {
alias(libs.plugins.jcstress)
}

dependencies {
implementation project(":micrometer-core")
// implementation("io.micrometer:micrometer-core:1.12.4")
runtimeOnly(libs.logbackLatest)
}

jcstress {
jcstressDependency 'org.openjdk.jcstress:jcstress-core:0.16'

verbose = true
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
/*
* 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.concurrencytests;

import io.micrometer.core.instrument.Counter;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
import org.openjdk.jcstress.annotations.*;
import org.openjdk.jcstress.infra.results.LL_Result;
import org.openjdk.jcstress.infra.results.Z_Result;

public class MeterRegistryConcurrencyTest {

/*
* Registering the same new Meter from multiple threads should be safe and consistent.
*/
@JCStressTest
@Outcome(id = { "true" }, expect = Expect.ACCEPTABLE, desc = "same meter returned for concurrent registers")
@Outcome(expect = Expect.FORBIDDEN)
@State
public static class ConcurrentRegisterNew {

MeterRegistry registry = new SimpleMeterRegistry();

Counter c1;

Counter c2;

@Actor
public void actor1() {
c1 = registry.counter("counter");
}

@Actor
public void actor2() {
c2 = registry.counter("counter");
}

@Arbiter
public void arbiter(Z_Result r) {
r.r1 = c1 == c2;
}

}

/*
* Likewise, registering an existing Meter from multiple threads is safe.
*/
@JCStressTest
@Outcome(id = { "true" }, expect = Expect.ACCEPTABLE, desc = "same meter returned for concurrent registers")
@Outcome(expect = Expect.FORBIDDEN)
@State
public static class ConcurrentRegisterExisting {

MeterRegistry registry = new SimpleMeterRegistry();

Counter c1;

Counter c2;

public ConcurrentRegisterExisting() {
registry.counter("counter");
}

@Actor
public void actor1() {
c1 = registry.counter("counter");
}

@Actor
public void actor2() {
c2 = registry.counter("counter");
}

@Arbiter
public void arbiter(Z_Result r) {
r.r1 = c1 == c2;
}

}

// @formatter:off
/*
When configuring a MeterFilter after a Meter has already been registered, existing meters will be marked stale.
Subsequent calls to {@code getOrCreateMeter} for those Meters create a new Meter with all MeterFilters applied.
If multiple concurrent calls to {@code getOrCreateMeter} interleave, it's possible not all see the new Meter.
We ideally want both to get the new meter, but we don't want to pay the cost associated with that level of safety
given the expected rarity of this situation happening, so we aim to get as close as possible for cheap.
RESULT SAMPLES FREQ EXPECT DESCRIPTION
null, null 0 0.00% Forbidden both get stale meter
null, tag 39,491 0.13% Interesting one stale meter returned
tag, null 40,389 0.13% Interesting one stale meter returned
tag, tag 30,941,328 99.74% Acceptable both get new meter
*/
// @formatter:on
@JCStressTest
@Outcome(id = { "tag, tag" }, expect = Expect.ACCEPTABLE, desc = "both get new meter")
@Outcome(id = { "null, tag", "tag, null" }, expect = Expect.ACCEPTABLE_INTERESTING,
desc = "one stale meter returned")
@Outcome(id = { "null, null" }, expect = Expect.FORBIDDEN, desc = "both get stale meter")
@State
public static class ConcurrentRegisterWithStaleId {

MeterRegistry registry = new SimpleMeterRegistry();

Counter c1;

Counter c2;

public ConcurrentRegisterWithStaleId() {
registry.counter("counter");
registry.counter("another");
registry.config().commonTags("common", "tag");
}

@Actor
public void actor1() {
c1 = registry.counter("counter");
}

@Actor
public void actor2() {
c2 = registry.counter("counter");
}

@Arbiter
public void arbiter(LL_Result r) {
r.r1 = c1.getId().getTag("common");
r.r2 = c2.getId().getTag("common");
}

}

}
35 changes: 35 additions & 0 deletions concurrency-tests/src/jcstress/resources/logback.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<!--
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.
-->
<configuration>

<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<!-- encoders are by default assigned the type
ch.qos.logback.classic.encoder.PatternLayoutEncoder -->
<encoder>
<pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
</encoder>
</appender>

<root level="warn">
<appender-ref ref="STDOUT" />
</root>

<!-- Late MeterFilter config logs are verbose in ConcurrentRegisterWithStaleId -->
<logger name="io.micrometer.core.instrument.simple.SimpleMeterRegistry" level="error"/>

</configuration>
1 change: 1 addition & 0 deletions config/checkstyle/checkstyle-suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<suppress checks="IllegalImport" files="docs[\\/].+" />

<suppress checks="JavadocPackageCheck" files="benchmarks[\\/].+" />
<suppress checks="JavadocPackageCheck" files="concurrency-tests[\\/].+" />
<suppress checks="JavadocPackageCheck" files="samples[\\/].+" />
<suppress checks="JavadocPackageCheck" files="[\\/]src[\\/]test[\\/].*" />

Expand Down
2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ javax-cacheApi = { module = "javax.cache:cache-api", version.ref = "javax-cache"
javax-inject = { module = "javax.inject:javax.inject", version.ref = "javax-inject" }
javax-servletApi = { module = "javax.servlet:javax.servlet-api", version = "4.0.1" }
jaxbApi = { module = "javax.xml.bind:jaxb-api", version.ref = "jaxb" }
jcstressCore = { module = "org.openjdk.jcstress:jcstress-core", version = "0.16" }
jetty9Client = { module = "org.eclipse.jetty:jetty-client", version.ref = "jetty9" }
jetty9Server = { module = "org.eclipse.jetty:jetty-server", version.ref = "jetty9" }
jetty9Servlet = { module = "org.eclipse.jetty:jetty-servlet", version.ref = "jetty9" }
Expand Down Expand Up @@ -231,3 +232,4 @@ plugin-bnd = "biz.aQute.bnd:biz.aQute.bnd.gradle:6.4.0"
[plugins]
kotlin19 = { id = "org.jetbrains.kotlin.jvm", version = "1.9.23" }
kotlin17 = { id = "org.jetbrains.kotlin.jvm", version = "1.7.22" }
jcstress = { id = "io.github.reyerizo.gradle.jcstress", version = "0.8.15" }
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,11 @@ public abstract class MeterRegistry {
*/
private final Map<Id, Meter> meterMap = new ConcurrentHashMap<>();

// writes guarded by meterMapLock, reads can be stale
private final Map<Id, Meter> preFilterIdToMeterMap = new HashMap<>();

private final Set<Id> stalePreFilterIds = ConcurrentHashMap.newKeySet();
// not thread safe; only needed when MeterFilter configured with Meters registered
private final Set<Id> stalePreFilterIds = new HashSet<>();

/**
* Map of meter id whose associated meter contains synthetic counterparts to those
Expand Down Expand Up @@ -619,7 +621,7 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
Function<Meter.Id, ? extends Meter> noopBuilder) {

Meter m = preFilterIdToMeterMap.get(originalId);
if (m != null && !unmarkStaleId(originalId)) {
if (m != null && !isStaleId(originalId)) {
return m;
}

Expand Down Expand Up @@ -661,13 +663,18 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
}
meterMap.put(mappedId, m);
preFilterIdToMeterMap.put(originalId, m);
unmarkStaleId(originalId);
}
}
}

return m;
}

private boolean isStaleId(Id originalId) {
return !stalePreFilterIds.isEmpty() && stalePreFilterIds.contains(originalId);
}

/**
* Marks the ID as no longer stale if it is stale. Otherwise, does nothing.
* @param originalId id before any filter mapping has been applied
Expand Down
Loading

0 comments on commit 858b9e7

Please sign in to comment.