SLING-12217 Cache model implementation class lookups per ResourceResolver

pom.xml

@@ -175,6 +175,11 @@
             <artifactId>junit-jupiter-api</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-params</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.junit.platform</groupId>
             <artifactId>junit-platform-engine</artifactId>

src/main/java/org/apache/sling/models/impl/AdapterImplementations.java

@@ -227,6 +227,25 @@ public <ModelType> ModelClass<ModelType> lookup(
         return null;
     }
 
+    /**
+     * Checks whether resolving the implementation for the given adapter type requires consulting the
+     * {@link ImplementationPicker}s. This is only the case when the adapter type is not directly registered as its own
+     * model (the cheap, deterministic fast path in {@link #lookup}) but has one or more implementations registered with
+     * a different adapter class. Only that path can cause repository access (resource-type based picking) and is
+     * therefore worth caching, see SLING-12217.
+     *
+     * @param adapterType the adapter type to check
+     * @return {@code true} if a picker has to be consulted to resolve the implementation
+     */
+    public boolean requiresImplementationPickerLookup(Class<?> adapterType) {
+        String key = adapterType.getName();
+        if (modelClasses.containsKey(key)) {
+            return false;
+        }
+        ConcurrentNavigableMap<String, ModelClass<?>> implementations = adapterImplementations.get(key);
+        return implementations != null && !implementations.isEmpty();
+    }
+
     /**
      * @param adapterType the type to check
      * @return {@code true} in case the given type is a model (may be with a different adapter class)

src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java

@@ -36,6 +36,7 @@
 import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -54,6 +55,7 @@
 import org.apache.sling.api.adapter.AdapterFactory;
 import org.apache.sling.api.adapter.AdapterManager;
 import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.models.annotations.Model;
 import org.apache.sling.models.annotations.ValidationStrategy;
 import org.apache.sling.models.annotations.ViaProviderType;
@@ -129,6 +131,30 @@ public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFacto
 
     private static final String REQUEST_CACHE_ATTRIBUTE = ModelAdapterFactory.class.getName() + ".AdapterCache";
 
+    // Implementation lookup cache:
+    // Resolving the implementation class for an adapter type is purely a function of the requested adapter type, the
+    // type of the adaptable and the resource type of the (possibly wrapped) resource. Only the picker-based path walks
+    // the resource super type hierarchy (repository access), so caching is limited to that path (see
+    // AdapterImplementations#requiresImplementationPickerLookup) and is scoped per ResourceResolver via its property
+    // map. This removes redundant lookups for the canCreateFromAdaptable/createModel pair and across components sharing
+    // a resource type, while leaving the cheap fast path untouched. The cache is a bounded LRU map (see
+    // implementationLookupCacheSize) so it cannot grow unbounded for long-lived resolvers or large numbers of distinct
+    // resource types. The cache key is "adaptableClass|adapterType|resourceType"; the concrete adaptable type is
+    // included so custom adaptables (e.g. request wrappers) do not share entries with unrelated ones.
+
+    /**
+     * Key under which the (bounded) implementation lookup cache is stored in the backing
+     * {@link ResourceResolver#getPropertyMap()} (see {@link #getImplementationTypeForAdapterType(Class, Object)}).
+     */
+    private static final String IMPLEMENTATION_LOOKUP_CACHE_KEY =
+            ModelAdapterFactory.class.getName() + ".ImplementationLookup";
+
+    /**
+     * Sentinel stored in the implementation lookup cache to memoize a negative result (no model implementation found
+     * for the given adaptable and resource type), so that repeated probing does not trigger the lookup again.
+     */
+    private static final Object NO_MODEL_IMPLEMENTATION = new Object();
+
     private final Logger log = LoggerFactory.getLogger(ModelAdapterFactory.class);
 
     private ReferenceQueue<Object> queue;
@@ -212,6 +238,12 @@ private void clearDisposalCallbackRegistryQueue() {
 
     private Map<Object, Map<Class<?>, SoftReference<Object>>> adapterCache;
 
+    /** Whether to cache model implementation class lookups per ResourceResolver, see SLING-12217. */
+    private boolean cacheImplementationLookups;
+
+    /** Maximum number of entries in the per-ResourceResolver implementation lookup cache, see SLING-12217. */
+    private int implementationLookupCacheSize;
+
     private SlingModelsScriptEngineFactory scriptEngineFactory;
 
     @Override
@@ -324,22 +356,102 @@ public boolean isModelClass(@NotNull Class<?> type) {
      *      href="http://sling.apache.org/documentation/bundles/models.html#specifying-an-alternate-adapter-class-since-sling-models-110">Specifying
      *      an Alternate Adapter Class</a>
      */
+    @SuppressWarnings("unchecked")
     private <ModelType> ModelClass<ModelType> getImplementationTypeForAdapterType(
             Class<ModelType> requestedType, Object adaptable) {
+        // only the (repository-accessing) picker-based path is cached
+        final boolean cacheable =
+                cacheImplementationLookups && adapterImplementations.requiresImplementationPickerLookup(requestedType);
+        final Map<String, Object> cache = cacheable ? getImplementationLookupCache(adaptable) : null;
+        final String cacheKey = cache != null ? getImplementationLookupCacheKey(requestedType, adaptable) : null;
+        if (cache != null && cacheKey != null) {
+            final Object cached = cache.get(cacheKey);
+            if (cached == NO_MODEL_IMPLEMENTATION) {
+                throw newNoImplementationException(requestedType, adaptable);
+            } else if (cached != null) {
+                return (ModelClass<ModelType>) cached;
+            }
+        }
+
         // lookup ModelClass wrapper for implementation type
         // additionally check if a different implementation class was registered for this adapter type
         // the adapter implementation is initially filled by the ModelPackageBundleList
         ModelClass<ModelType> modelClass =
                 this.adapterImplementations.lookup(requestedType, adaptable, this.implementationPickers);
+
+        if (cache != null && cacheKey != null) {
+            cache.put(cacheKey, modelClass != null ? modelClass : NO_MODEL_IMPLEMENTATION);
+        }
+
         if (modelClass != null) {
             log.debug("Using implementation type {} for requested adapter type {}", modelClass, requestedType);
             return modelClass;
         }
         // throw exception here
-        throw new ModelClassException("Could not yet find an adapter factory for the model " + requestedType
+        throw newNoImplementationException(requestedType, adaptable);
+    }
+
+    private ModelClassException newNoImplementationException(Class<?> requestedType, Object adaptable) {
+        return new ModelClassException("Could not yet find an adapter factory for the model " + requestedType
                 + " from adaptable " + adaptable.getClass());
     }
 
+    /**
+     * Returns the (resolver scoped) cache for implementation lookups, lazily creating it in the
+     * {@link ResourceResolver#getPropertyMap() property map} of the resolver backing the given adaptable. Returns
+     * {@code null} if no resource (and hence no resolver) can be derived from the adaptable.
+     *
+     * <p>The cache is a bounded LRU map: even for long-lived (e.g. service) resolvers or pathological numbers of
+     * distinct resource types, it never holds more than {@link #implementationLookupCacheSize} entries, evicting the
+     * least recently used ones.
+     */
+    @SuppressWarnings("unchecked")
+    private @Nullable Map<String, Object> getImplementationLookupCache(Object adaptable) {
+        final Resource resource = getResourceFromAdaptable(adaptable);
+        if (resource == null) {
+            return null;
+        }
+        final ResourceResolver resolver = resource.getResourceResolver();
+        final Map<String, Object> propertyMap = resolver.getPropertyMap();
+
+        return (Map<String, Object>) propertyMap.computeIfAbsent(
+                IMPLEMENTATION_LOOKUP_CACHE_KEY, v -> createBoundedLookupCache(implementationLookupCacheSize));
+    }
+
+    private static Map<String, Object> createBoundedLookupCache(final int maxSize) {
+        return Collections.synchronizedMap(new LinkedHashMap<String, Object>(16, 0.75f, true) {
+            @Override
+            protected boolean removeEldestEntry(Map.Entry<String, Object> eldest) {
+                return size() > maxSize;
+            }
+        });
+    }
+
+    /**
+     * Builds the cache key for an implementation lookup, or {@code null} if no resource type is available, in which case
+     * the lookup must not be cached.
+     */
+    private @Nullable String getImplementationLookupCacheKey(Class<?> requestedType, Object adaptable) {
+        final Resource resource = getResourceFromAdaptable(adaptable);
+        if (resource == null) {
+            return null;
+        }
+        final String resourceType = resource.getResourceType();
+        return adaptable.getClass().getName() + '|' + requestedType.getName() + '|' + resourceType;
+    }
+
+    @SuppressWarnings("deprecation")
+    private @Nullable Resource getResourceFromAdaptable(Object adaptable) {
+        if (adaptable instanceof Resource resource) {
+            return resource;
+        } else if (adaptable instanceof SlingJakartaHttpServletRequest jakartaRequest) {
+            return jakartaRequest.getResource();
+        } else if (adaptable instanceof SlingHttpServletRequest javaxRequest) {
+            return javaxRequest.getResource();
+        }
+        return null;
+    }
+
     @SuppressWarnings("unchecked")
     private Map<Class<?>, SoftReference<Object>> getOrCreateCache(final Object adaptable) {
         Map<Class<?>, SoftReference<Object>> adaptableCache;
@@ -1186,6 +1298,9 @@ protected ThreadInvocationCounter initialValue() {
         this.adapterCache =
                 Collections.synchronizedMap(new WeakHashMap<Object, Map<Class<?>, SoftReference<Object>>>());
 
+        this.cacheImplementationLookups = configuration.cache_implementation_lookups();
+        this.implementationLookupCacheSize = Math.max(1, configuration.implementation_lookup_cache_size());
+
         BundleContext bundleContext = ctx.getBundleContext();
         this.queue = new ReferenceQueue<>();
         this.disposalCallbacks = new ConcurrentHashMap<>();

src/main/java/org/apache/sling/models/impl/ModelAdapterFactoryConfiguration.java

@@ -30,4 +30,21 @@
             name = "Cleanup Job Period",
             description = "Period in seconds at which OSGi service references from ThreadLocals will be cleaned up.")
     long cleanup_job_period() default 30l;
+
+    @AttributeDefinition(
+            name = "Cache Implementation Lookups",
+            description = "Cache the resolution of a model implementation class per adaptable type and resource type "
+                    + "within a ResourceResolver. This avoids redundant repository lookups when the same model is "
+                    + "resolved repeatedly (e.g. once for canCreateFromAdaptable and once for createModel, or across "
+                    + "many components sharing a resource type). Disable this if a custom ImplementationPicker selects "
+                    + "an implementation based on more than the resource type (e.g. selectors or request attributes).")
+    boolean cache_implementation_lookups() default true;
+
+    @AttributeDefinition(
+            name = "Implementation Lookup Cache Size",
+            description = "Maximum number of entries in the per-ResourceResolver implementation lookup cache (see "
+                    + "'Cache Implementation Lookups'). Once exceeded, the least recently used entries are evicted, so "
+                    + "that the cache cannot grow unbounded for long-lived resolvers or large numbers of distinct "
+                    + "resource types. Must be greater than 0.")
+    int implementation_lookup_cache_size() default 100;
 }

src/test/java/org/apache/sling/models/impl/AdapterFactoryTest.java

@@ -80,13 +80,23 @@ public static ModelAdapterFactory createModelAdapterFactory() {
         return createModelAdapterFactory(bundleContext);
     }
 
+    public static ModelAdapterFactory createModelAdapterFactory(boolean cacheImplementationLookups) {
+        return createModelAdapterFactory(Mockito.mock(BundleContext.class), cacheImplementationLookups);
+    }
+
     public static ModelAdapterFactory createModelAdapterFactory(BundleContext bundleContext) {
+        return createModelAdapterFactory(bundleContext, true);
+    }
+
+    public static ModelAdapterFactory createModelAdapterFactory(
+            BundleContext bundleContext, boolean cacheImplementationLookups) {
         ComponentContext componentCtx = Mockito.mock(ComponentContext.class);
         when(componentCtx.getBundleContext()).thenReturn(bundleContext);
 
         ModelAdapterFactory factory = new ModelAdapterFactory();
         Converter c = Converters.standardConverter();
         Map<String, String> map = new HashMap<>();
+        map.put("cache.implementation.lookups", Boolean.toString(cacheImplementationLookups));
         ModelAdapterFactoryConfiguration config = c.convert(map).to(ModelAdapterFactoryConfiguration.class);
         factory.activate(componentCtx, config);
         factory.injectAnnotationProcessorFactories = Collections.emptyList();

src/test/java/org/apache/sling/models/impl/CachingTest.java

@@ -39,6 +39,9 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.Parameter;
+import org.junit.jupiter.params.ParameterizedClass;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.Mock;
 import org.mockito.Spy;
 import org.mockito.junit.jupiter.MockitoExtension;
@@ -50,9 +53,16 @@
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
+// run with the implementation-lookup cache (SLING-12217) both enabled and disabled - the model instance caching
+// behavior asserted here must be identical regardless of that optimization
+@ParameterizedClass
+@ValueSource(booleans = {true, false})
 @ExtendWith(MockitoExtension.class)
 class CachingTest {
 
+    @Parameter
+    private boolean cacheImplementationLookups;
+
     @Spy
     private MockSlingJakartaHttpServletRequest request = new MockSlingJakartaHttpServletRequest(null);
 
@@ -65,7 +75,7 @@ class CachingTest {
 
     @BeforeEach
     void setup() {
-        factory = AdapterFactoryTest.createModelAdapterFactory();
+        factory = AdapterFactoryTest.createModelAdapterFactory(cacheImplementationLookups);
         factory.injectors = Arrays.asList(new RequestAttributeInjector(), new ValueMapInjector());
         factory.adapterImplementations.addClassesAsAdapterAndImplementation(
                 CachedModel.class,