From b95485d662f5b4c638c6bb1e38105375759b12b9 Mon Sep 17 00:00:00 2001 From: zanarelli Date: Sat, 1 Aug 2026 01:16:57 -0300 Subject: [PATCH] Share projection metadata read across factory instances. DefaultProjectionInformation.PropertyDescriptorSource.getMetadata() performed a fresh ASM MetadataReader classpath read of a projection interface's class file on every construction, and ProxyProjectionFactory.projectionInformationCache is per-instance. Two independent ProxyProjectionFactory/SpelAwareProxyProjectionFactory instances resolving the same projection type each paid their own class file read, with no sharing. In reactive applications this shows up as a Blockhound-flagged blocking file read on the event loop. Add a shared, type-keyed ConcurrentReferenceHashMap (weak keys) cache for the metadata read, below the per-factory layer. Closes #3513 Signed-off-by: zanarelli --- .../DefaultProjectionInformation.java | 17 ++ .../projection/GH3513SampleProjection.java | 30 +++ .../ProjectionMetadataCachingProbeTests.java | 174 ++++++++++++++++++ 3 files changed, 221 insertions(+) create mode 100644 src/test/java/org/springframework/data/projection/GH3513SampleProjection.java create mode 100644 src/test/java/org/springframework/data/projection/ProjectionMetadataCachingProbeTests.java diff --git a/src/main/java/org/springframework/data/projection/DefaultProjectionInformation.java b/src/main/java/org/springframework/data/projection/DefaultProjectionInformation.java index a3237a2577..901e7d4744 100644 --- a/src/main/java/org/springframework/data/projection/DefaultProjectionInformation.java +++ b/src/main/java/org/springframework/data/projection/DefaultProjectionInformation.java @@ -39,6 +39,8 @@ import org.springframework.data.util.StreamUtils; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.ConcurrentReferenceHashMap; +import org.springframework.util.ConcurrentReferenceHashMap.ReferenceType; /** * Default implementation of {@link ProjectionInformation}. Exposes all properties of the type as required input @@ -48,6 +50,7 @@ * @author Christoph Strobl * @author Mark Paluch * @author Johannes Englmeier + * @author Raphael Zanarelli * @since 1.12 */ class DefaultProjectionInformation implements ProjectionInformation { @@ -125,6 +128,16 @@ private static class PropertyDescriptorSource { private static final Log logger = LogFactory.getLog(PropertyDescriptorSource.class); + /** + * Cache for the {@link AnnotationMetadata} read for a given type, shared across {@link PropertyDescriptorSource} + * (and therefore {@link DefaultProjectionInformation}/{@link ProjectionFactory}) instances. The underlying class + * file read via {@link MetadataReader} is purely type-derived and does not depend on the requesting factory, so + * unlike {@link ProjectionInformation} itself it is safe, and desirable, to memoize below the per-factory layer. + * Weak keys avoid pinning classes/class loaders that would otherwise become unreachable. + */ + private static final Map, Optional> METADATA_CACHE = new ConcurrentReferenceHashMap<>( + 256, ReferenceType.WEAK); + private final Class type; private final Optional metadata; @@ -219,6 +232,10 @@ private Stream> fromType() { * @return the optional {@link AnnotationMetadata}. */ private static Optional getMetadata(Class type) { + return METADATA_CACHE.computeIfAbsent(type, PropertyDescriptorSource::readMetadata); + } + + private static Optional readMetadata(Class type) { try { diff --git a/src/test/java/org/springframework/data/projection/GH3513SampleProjection.java b/src/test/java/org/springframework/data/projection/GH3513SampleProjection.java new file mode 100644 index 0000000000..fbd80f76c4 --- /dev/null +++ b/src/test/java/org/springframework/data/projection/GH3513SampleProjection.java @@ -0,0 +1,30 @@ +/* + * Copyright 2026-present 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 org.springframework.data.projection; + +/** + * Standalone projection interface used by {@link ProjectionMetadataCachingProbeTests} to reproduce GH-3513. Kept as + * its own top-level type (rather than nested) so its {@code .class} file can be loaded independently through an + * isolated {@link ClassLoader} without dragging in an enclosing class. + * + * @author Raphael Zanarelli + */ +public interface GH3513SampleProjection { + + String getFirstname(); + + String getLastname(); +} diff --git a/src/test/java/org/springframework/data/projection/ProjectionMetadataCachingProbeTests.java b/src/test/java/org/springframework/data/projection/ProjectionMetadataCachingProbeTests.java new file mode 100644 index 0000000000..8d1b414e0c --- /dev/null +++ b/src/test/java/org/springframework/data/projection/ProjectionMetadataCachingProbeTests.java @@ -0,0 +1,174 @@ +/* + * Copyright 2026-present 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 org.springframework.data.projection; + +import static org.assertj.core.api.Assertions.*; + +import java.io.IOException; +import java.io.InputStream; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.Test; + +/** + * Probe reproducing GH-3513: {@link DefaultProjectionInformation} reads the projection type's class-file metadata + * (via ASM {@code MetadataReader}) once per {@link ProxyProjectionFactory} instance instead of sharing that read + * across factory instances. In reactive applications this shows up as a Blockhound-flagged blocking file read + * happening again on the event loop, because the request-time factory (e.g. inside a converter) never benefits from + * the read already performed on the boot thread by a different factory instance. + *

+ * This test does not require Blockhound/R2DBC to demonstrate the underlying defect: it counts how many times the + * projection type's {@code .class} resource is actually read from the classpath while resolving + * {@link ProjectionInformation} through two independent, otherwise identically configured + * {@link SpelAwareProxyProjectionFactory} instances - mirroring the "bootstrap factory" vs. "converter's own factory" + * situation described in the issue. + * + * @author Raphael Zanarelli + */ +class ProjectionMetadataCachingProbeTests { + + private static final String PROJECTION_CLASS_NAME = GH3513SampleProjection.class.getName(); + + @Test // GH-3513 + void sharesClassMetadataReadWithinSingleFactoryInstance() throws Exception { + + ReadCountingClassLoader loader = new ReadCountingClassLoader(getClass().getClassLoader()); + Class isolatedType = loader.loadIsolated(PROJECTION_CLASS_NAME); + + SpelAwareProxyProjectionFactory factory = new SpelAwareProxyProjectionFactory(); + + factory.getProjectionInformation(isolatedType); + factory.getProjectionInformation(isolatedType); + + // Same factory: ProxyProjectionFactory#projectionInformationCache (computeIfAbsent) already avoids a + // second read for a repeated lookup on the same instance - this must hold both before and after any fix. + assertThat(loader.getReadCount()).isEqualTo(1); + } + + @Test // GH-3513 + void reproducesRedundantClassMetadataReadAcrossIndependentFactoryInstances() throws Exception { + + ReadCountingClassLoader loader = new ReadCountingClassLoader(getClass().getClassLoader()); + Class isolatedType = loader.loadIsolated(PROJECTION_CLASS_NAME); + + // Mirrors the reported scenario: one factory resolves the projection at bootstrap (e.g. the repository's + // own SpelAwareProxyProjectionFactory), a second, independent factory instance resolves the very same + // projection type again later (e.g. a converter's private factory). + SpelAwareProxyProjectionFactory bootstrapFactory = new SpelAwareProxyProjectionFactory(); + SpelAwareProxyProjectionFactory requestTimeFactory = new SpelAwareProxyProjectionFactory(); + + bootstrapFactory.getProjectionInformation(isolatedType); + + assertThat(loader.getReadCount()) // + .describedAs("expected exactly one class-file read after the first factory resolved the projection") // + .isEqualTo(1); + + requestTimeFactory.getProjectionInformation(isolatedType); + + // With the fix (a shared, factory-independent cache for the metadata read) this stays at 1: the second + // factory should be able to reuse the read already performed by the first one for the very same type. + assertThat(loader.getReadCount()) // + .describedAs("second, independent factory instance should reuse the already-read class metadata") // + .isEqualTo(1); + } + + /** + * A {@link ClassLoader} that defines exactly one class itself (bypassing parent delegation for that single name) + * so that {@code type.getClassLoader()} returns this instance, and counts every time that class's {@code .class} + * resource is actually read from the classpath. + */ + private static class ReadCountingClassLoader extends ClassLoader { + + private final AtomicInteger reads = new AtomicInteger(); + private String isolatedClassName; + private String isolatedResourcePath; + + ReadCountingClassLoader(ClassLoader parent) { + super(parent); + } + + Class loadIsolated(String className) throws ClassNotFoundException { + + this.isolatedClassName = className; + this.isolatedResourcePath = className.replace('.', '/') + ".class"; + + return loadClass(className); + } + + int getReadCount() { + return reads.get(); + } + + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + + if (!name.equals(isolatedClassName)) { + return super.loadClass(name, resolve); + } + + synchronized (getClassLoadingLock(name)) { + + Class loaded = findLoadedClass(name); + + if (loaded == null) { + loaded = findClass(name); + } + + if (resolve) { + resolveClass(loaded); + } + + return loaded; + } + } + + @Override + protected Class findClass(String name) throws ClassNotFoundException { + + if (!name.equals(isolatedClassName)) { + return super.findClass(name); + } + + byte[] bytes = readClassBytes(); + return defineClass(name, bytes, 0, bytes.length); + } + + private byte[] readClassBytes() throws ClassNotFoundException { + + try (InputStream in = getParent().getResourceAsStream(isolatedResourcePath)) { + + if (in == null) { + throw new ClassNotFoundException(isolatedClassName); + } + + return in.readAllBytes(); + + } catch (IOException e) { + throw new ClassNotFoundException(isolatedClassName, e); + } + } + + @Override + public InputStream getResourceAsStream(String name) { + + if (name.equals(isolatedResourcePath)) { + reads.incrementAndGet(); + } + + return getParent().getResourceAsStream(name); + } + } +}