[ 
https://issues.apache.org/jira/browse/SLING-8759?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17331018#comment-17331018
 ] 

Joerg Hoh edited comment on SLING-8759 at 4/23/21, 8:32 PM:
------------------------------------------------------------

I created SLING-10327 to handle the refactoring of getAdapter(), so we can keep 
this one open. Unfortunately the Git comments reference this one already (and 
rewriting them now doesn't seem to be the best idea).

[~rombert] do you have a good idea how avoid the mandatory memory overhead 
while keeping the ConcurrentMap approach? If not, I suggest to release Sling 
API 2.23.4 without SLING-8759.


was (Author: joerghoh):
I created SLING-10327 to handle the refactoring of getAdapter(), so we can keep 
this one open. Unfortunately the Git comments reference this one already (and 
rewriting them now doesn't seem to be the best idea).

> Replace the SlingAdaptable.adapterCache with a ConcurrentMap
> ------------------------------------------------------------
>
>                 Key: SLING-8759
>                 URL: https://issues.apache.org/jira/browse/SLING-8759
>             Project: Sling
>          Issue Type: Improvement
>          Components: API
>            Reporter: Robert Munteanu
>            Assignee: Joerg Hoh
>            Priority: Major
>             Fix For: API 2.23.4
>
>          Time Spent: 1h 50m
>  Remaining Estimate: 0h
>
> 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.
> {code}
> 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);
> +        });
>      }
>  }
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to