[SLING-8759] Replace the SlingAdaptable.adapterCache with a ConcurrentMap

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Won't Fix
• Fix Version/s: API 2.23.4
• Component/s: API
• Assignee: Joerg Hoh
• Reporter: Robert Munteanu
• Created: 02/Oct/19 13:04
• Updated: 12/May/21 06:32
• Resolved: 26/Apr/21 07:55

Description

The SlingAdaptable.adapterCache is based on a HashMap guarded by a synchronized block, with lazy initialisation. This looks like a perfect scenario for ConcurrentMap.computeIfAbsent. The increased memory usage should be measured though, as a simple implementation will not use lazy initialisation, e.g.

diff --git a/src/main/java/org/apache/sling/api/adapter/SlingAdaptable.java b/src/main/java/org/apache/sling/api/adapter/SlingAdaptable.java
index 5adf0ce..6bed14f 100644
--- a/src/main/java/org/apache/sling/api/adapter/SlingAdaptable.java
+++ b/src/main/java/org/apache/sling/api/adapter/SlingAdaptable.java
@@ -20,6 +20,8 @@ package org.apache.sling.api.adapter;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 /**
  * The <code>SlingAdaptable</code> class is an (abstract) default implementation
@@ -74,7 +76,7 @@ public abstract class SlingAdaptable implements Adaptable {
      * are intended to be short-lived to not hold on to objects and classes for
      * too long.
      */
-    private Map<Class<?>, Object> adaptersCache;
+    private ConcurrentMap<Class<?>, Object> adaptersCache = new ConcurrentHashMap<Class<?>, Object>();
 
     /**
      * Calls into the registered {@link AdapterManager} to adapt this object to
@@ -94,22 +96,8 @@ public abstract class SlingAdaptable implements Adaptable {
      */
     @SuppressWarnings("unchecked")
     public <AdapterType> AdapterType adaptTo(Class<AdapterType> type) {
-        AdapterType result = null;
-        synchronized ( this ) {
-            if ( adaptersCache != null ) {
-                result = (AdapterType) adaptersCache.get(type);
-            }
-            if ( result == null ) {
-                final AdapterManager mgr = ADAPTER_MANAGER;
-                result = (mgr == null ? null : mgr.getAdapter(this, type));
-                if ( result != null ) {
-                    if ( adaptersCache == null ) {
-                        adaptersCache = new HashMap<Class<?>, Object>();
-                    }
-                    adaptersCache.put(type, result);
-                }
-            }
-        }
-        return result;
+        return (AdapterType) adaptersCache.computeIfAbsent(type, (klazz) -> {
+            return ADAPTER_MANAGER.getAdapter(this, type);
+        });
     }
 }

Issue Links

• relates to: SLING-10327 SlingAdaptable: simplify getAdapter()