jsedding commented on code in PR #236:
URL: 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/236#discussion_r4193751123


##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java:
##########
@@ -1148,17 +1148,14 @@ public boolean hasChildren(Resource resource) {
     @Override
     public String getParentResourceType(final Resource resource) {
         checkClosed();
-        String resourceSuperType = null;
-        if (resource != null) {
-            if (parentResourceTypeMap.containsKey(resource.getPath())) {
-                resourceSuperType =
-                        
parentResourceTypeMap.get(resource.getPath()).orElse(null);
-            } else {
-                resourceSuperType = getParentResourceTypeInternal(resource);
-                parentResourceTypeMap.put(resource.getPath(), 
Optional.ofNullable(resourceSuperType));
-            }
+        if (resource == null) {
+            return null;
         }
-        return resourceSuperType;
+        final ResourceTypeInformation key =
+                new ResourceTypeInformation(resource.getResourceType(), 
resource.getResourceSuperType(), null);
+        return parentResourceTypeMap
+                .computeIfAbsent(key, k -> 
Optional.ofNullable(getParentResourceTypeInternal(resource)))
+                .orElse(null);

Review Comment:
   I wonder if it is not sufficient to cache by `resource.getResourceType()`? 
The code in `getParentResourceTypeInternal(...)` only calls 
`this.getParentResourceType(...)` if `resource.getResourceSuperType()` is 
`null`. We could opt to only cache results of `this.getParentResourceType(...)`.
   
   This would work fine, as long as we can assume that 
`resource.getResourceSuperType()` is cheap. My assumption would be that it is 
in fact cheap, and for the common case of JCR resources, it _should_ be cached 
in the `ValueMap` if accessed repeatedly.
   
   The benefit of structuring the caching like that would be that the cache 
holds less entries and the in-memory size of the keys is smaller (a String). 
Both would lead to lower heap usage.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to