From 615ce93f8eaaca9b47ff88f2082b6ae71f6fb8cc Mon Sep 17 00:00:00 2001 From: yawkat Date: Tue, 23 Jul 2024 09:39:35 +0200 Subject: [PATCH] Add type thrashing test Simple test that does back-and-forth serialization and looks for type thrashing. Also contains a fix for the DefaultSerdeRegistry cache map to avoid the thrashing. Other than that this seems to be a non-issue. --- benchmarks/build.gradle | 29 +++++++++ .../io/micronaut/serde/ComboBenchmark.java | 60 ++++++++++++++++++ .../io/micronaut/serde/TypeThrashingTest.java | 61 +++++++++++++++++++ gradle/libs.versions.toml | 2 +- .../serde/support/DefaultSerdeRegistry.java | 19 +++--- 5 files changed, 163 insertions(+), 8 deletions(-) create mode 100644 benchmarks/src/jmh/java/io/micronaut/serde/ComboBenchmark.java create mode 100644 benchmarks/src/typeCheckTest/java/io/micronaut/serde/TypeThrashingTest.java diff --git a/benchmarks/build.gradle b/benchmarks/build.gradle index 05ed4d547..e65a7177a 100644 --- a/benchmarks/build.gradle +++ b/benchmarks/build.gradle @@ -5,6 +5,13 @@ plugins { id("me.champeau.jmh") version "0.7.2" } +sourceSets { + create("typeCheckTest") { + compileClasspath += sourceSets.jmh.output + runtimeClasspath += sourceSets.jmh.output + } +} + dependencies { annotationProcessor(projects.micronautSerdeProcessor) annotationProcessor(mn.micronaut.inject.java) @@ -19,6 +26,16 @@ dependencies { jmh(libs.jmh.core) runtimeOnly(mnLogging.logback.classic) + + typeCheckTestImplementation(mnTest.junit.jupiter.engine) + typeCheckTestImplementation(mnTest.micronaut.test.type.pollution) + typeCheckTestImplementation(mnTest.bytebuddy) + typeCheckTestImplementation(mnTest.bytebuddy.agent) +} + +configurations { + typeCheckTestImplementation.extendsFrom(jmhImplementation, implementation) + typeCheckTestRuntimeOnly.extendsFrom(jmhRuntimeOnly, runtimeOnly) } jmh { @@ -29,6 +46,18 @@ jmh { duplicateClassesStrategy = DuplicatesStrategy.EXCLUDE } +tasks.register('typeCheckTest', Test) { + description = "Runs type check tests." + group = "verification" + + testClassesDirs = sourceSets.typeCheckTest.output.classesDirs + classpath = sourceSets.typeCheckTest.runtimeClasspath + + useJUnitPlatform() +} + +check.dependsOn typeCheckTest + shadowJar { mergeServiceFiles() } diff --git a/benchmarks/src/jmh/java/io/micronaut/serde/ComboBenchmark.java b/benchmarks/src/jmh/java/io/micronaut/serde/ComboBenchmark.java new file mode 100644 index 000000000..ffdff4d15 --- /dev/null +++ b/benchmarks/src/jmh/java/io/micronaut/serde/ComboBenchmark.java @@ -0,0 +1,60 @@ +package io.micronaut.serde; + +import io.micronaut.context.ApplicationContext; +import io.micronaut.core.type.Argument; +import io.micronaut.json.JsonMapper; +import io.micronaut.serde.data.StringArrayField; +import io.micronaut.serde.jackson.JacksonJsonMapper; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.infra.Blackhole; + +import java.io.IOException; + +public class ComboBenchmark { + + @State(Scope.Thread) + public static class Holder { + @Param + TestCase testCase; + + JsonMapper jsonMapper; + ApplicationContext ctx; + + @Setup + public void setUp() { + ctx = ApplicationContext.run(); + jsonMapper = ctx.getBean(JacksonJsonMapper.class).createSpecific(testCase.type); + } + + @TearDown + public void tearDown() { + ctx.close(); + } + } + + public enum TestCase { + STRING_ARRAY_FIELD(Argument.of(StringArrayField.class), new StringArrayField() {{ strs = new String[] { "foo", "bar" }; }}), + STRING(Argument.of(String.class), "foo"); + + final Argument type; + final Object input; + + TestCase(Argument type, Object input) { + this.type = type; + this.input = input; + } + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + @Benchmark + public Object serde(Holder holder, Blackhole bh) throws IOException { + byte[] bytes = holder.jsonMapper.writeValueAsBytes((Argument) holder.testCase.type, holder.testCase.input); + bh.consume(bytes); + return holder.jsonMapper.readValue(bytes, holder.testCase.type); + } +} diff --git a/benchmarks/src/typeCheckTest/java/io/micronaut/serde/TypeThrashingTest.java b/benchmarks/src/typeCheckTest/java/io/micronaut/serde/TypeThrashingTest.java new file mode 100644 index 000000000..dab144a69 --- /dev/null +++ b/benchmarks/src/typeCheckTest/java/io/micronaut/serde/TypeThrashingTest.java @@ -0,0 +1,61 @@ +package io.micronaut.serde; + +import io.micronaut.test.typepollution.FocusListener; +import io.micronaut.test.typepollution.ThresholdFocusListener; +import io.micronaut.test.typepollution.TypePollutionTransformer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openjdk.jmh.annotations.Mode; +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; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +public class TypeThrashingTest { + static final int THRESHOLD = 10_000; + + private ThresholdFocusListener focusListener; + + @BeforeAll + static void setupAgent() { + TypePollutionTransformer.install(net.bytebuddy.agent.ByteBuddyAgent.install()); + } + + @BeforeEach + void setUp() { + focusListener = new ThresholdFocusListener(); + FocusListener.setFocusListener(focusListener); + } + + @AfterEach + void verifyNoTypeThrashing() { + FocusListener.setFocusListener(null); + Assertions.assertTrue(focusListener.checkThresholds(THRESHOLD), "Threshold exceeded, check logs."); + } + + @Test + public void testFromJmh() throws RunnerException { + Options opt = new OptionsBuilder() + .include(Stream.of(ComboBenchmark.class) + .map(Class::getName) + .collect(Collectors.joining("|", "(", ")")) + + ".*") + .warmupIterations(0) + .measurementIterations(1) + .mode(Mode.SingleShotTime) + .timeUnit(TimeUnit.NANOSECONDS) + .forks(0) + .measurementBatchSize(THRESHOLD * 2) + .shouldFailOnError(true) + .build(); + + new Runner(opt).run(); + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 784ed6ed5..25e0fa9c5 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -18,7 +18,7 @@ groovy = "4.0.18" micronaut = "4.5.3" micronaut-platform = "4.4.3" micronaut-docs = "2.0.0" -micronaut-test = "4.3.0" +micronaut-test = "4.4.0" micronaut-discovery = "4.3.0" micronaut-logging = "1.2.3" micronaut-oracle-cloud = "3.8.0" diff --git a/serde-support/src/main/java/io/micronaut/serde/support/DefaultSerdeRegistry.java b/serde-support/src/main/java/io/micronaut/serde/support/DefaultSerdeRegistry.java index b0b8e3738..5b1e225d9 100644 --- a/serde-support/src/main/java/io/micronaut/serde/support/DefaultSerdeRegistry.java +++ b/serde-support/src/main/java/io/micronaut/serde/support/DefaultSerdeRegistry.java @@ -229,7 +229,10 @@ public class DefaultSerdeRegistry implements SerdeRegistry { private final List> deserializers = new ArrayList<>(100); private final List> internalSerdes = new ArrayList<>(100); - private final Map> serializerMap = new ConcurrentHashMap<>(50); + // if there is a single Serde that is part of the serializerMap *and* deserializerMap, this can + // lead to interface type check thrashing. For that reason, we wrap the serializer side with + // a wrapper object. + private final Map serializerMap = new ConcurrentHashMap<>(50); private final Map> deserializerMap = new ConcurrentHashMap<>(50); private final BeanContext beanContext; @@ -501,9 +504,9 @@ public Collection> getDeserializableSubtypes( public Serializer findSerializer(Argument type) throws SerdeException { Objects.requireNonNull(type, "Type cannot be null"); final TypeKey key = new TypeKey(type); - final Serializer serializer = serializerMap.get(key); - if (serializer != null) { - return (Serializer) serializer; + SerializerWrapper wrapper = serializerMap.get(key); + if (wrapper != null) { + return (Serializer) wrapper.serializer; } if (type.getType().equals(Object.class)) { return objectSerializer; @@ -522,14 +525,14 @@ public Serializer findSerializer(Argument type) thro ser = getBean(definition); } if (ser != null) { - serializerMap.put(key, ser); + serializerMap.put(key, new SerializerWrapper(ser)); return (Serializer) ser; } if (key.getType().isArray()) { - serializerMap.put(key, objectArraySerde); + serializerMap.put(key, new SerializerWrapper(objectArraySerde)); return (Serializer) objectArraySerde; } - serializerMap.put(key, objectSerializer); + serializerMap.put(key, new SerializerWrapper(objectSerializer)); return objectSerializer; } @@ -714,4 +717,6 @@ public String toString() { } + private record SerializerWrapper(Serializer serializer) { + } }