Uploaded image for project: 'Sling'
  1. Sling
  2. SLING-8759

Replace the SlingAdaptable.adapterCache with a ConcurrentMap

    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Won't Fix
    • None
    • API 2.23.4
    • API
    • None

    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);
      +        });
           }
       }
      

      Attachments

        Issue Links

          Activity

            People

              joerghoh Joerg Hoh
              rombert Robert Munteanu
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 2h 40m
                  2h 40m