Author: rombert
Date: Wed Sep 17 08:56:19 2014
New Revision: 1625490

URL: http://svn.apache.org/r1625490
Log:
SLING-3936 - Poor performance when adding a large (?) number of
sling.auth.requirements services

Replace ArrayList + explicit sort with TreeSet.

Modified:
    
sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/PathBasedHolderCache.java
    
sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java

Modified: 
sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/PathBasedHolderCache.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/PathBasedHolderCache.java?rev=1625490&r1=1625489&r2=1625490&view=diff
==============================================================================
--- 
sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/PathBasedHolderCache.java
 (original)
+++ 
sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/PathBasedHolderCache.java
 Wed Sep 17 08:56:19 2014
@@ -19,17 +19,19 @@
 package org.apache.sling.auth.core.impl;
 
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.SortedSet;
+import java.util.TreeSet;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import javax.servlet.http.HttpServletRequest;
 
 public class PathBasedHolderCache<Type extends PathBasedHolder> {
 
-    private final Map<String, Map<String, List<Type>>> cache = new 
HashMap<String, Map<String, List<Type>>>();
+    private final Map<String, Map<String, SortedSet<Type>>> cache = new 
HashMap<String, Map<String, SortedSet<Type>>>();
 
     /** Read/write lock to synchronize the cache access. */
     private final ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock();
@@ -47,28 +49,25 @@ public class PathBasedHolderCache<Type e
         this.rwLock.writeLock().lock();
         try {
 
-            Map<String, List<Type>> byHostMap = cache.get(holder.protocol);
+            Map<String, SortedSet<Type>> byHostMap = 
cache.get(holder.protocol);
             if (byHostMap == null) {
-                byHostMap = new HashMap<String, List<Type>>();
+                byHostMap = new HashMap<String, SortedSet<Type>>();
                 cache.put(holder.protocol, byHostMap);
             }
 
-            final List<Type> byPathList = new ArrayList<Type>();
+            final SortedSet<Type> byPathSet = new TreeSet<Type>();
 
             // preset with current list
-            final List<Type> currentPathList = byHostMap.get(holder.host);
-            if (currentPathList != null) {
-                byPathList.addAll(currentPathList);
+            final SortedSet<Type> currentPathSet = byHostMap.get(holder.host);
+            if (currentPathSet != null) {
+                byPathSet.addAll(currentPathSet);
             }
 
             // add the new holder
-            byPathList.add(holder);
+            byPathSet.add(holder);
 
-            // sort the list according to the path length (longest path first)
-            Collections.sort(byPathList);
-
-            // replace old list with new list
-            byHostMap.put(holder.host, byPathList);
+            // replace old set with new set
+            byHostMap.put(holder.host, byPathSet);
         } finally {
             this.rwLock.writeLock().unlock();
         }
@@ -77,21 +76,21 @@ public class PathBasedHolderCache<Type e
     public void removeHolder(final Type holder) {
         this.rwLock.writeLock().lock();
         try {
-            final Map<String, List<Type>> byHostMap = 
cache.get(holder.protocol);
+            final Map<String, SortedSet<Type>> byHostMap = 
cache.get(holder.protocol);
             if (byHostMap != null) {
-                final List<Type> byPathList = byHostMap.get(holder.host);
-                if (byPathList != null) {
+                final SortedSet<Type> byPathSet = byHostMap.get(holder.host);
+                if (byPathSet != null) {
 
-                    // create a new list without the removed holder
-                    final List<Type> list = new ArrayList<Type>();
-                    list.addAll(byPathList);
-                    list.remove(holder);
+                    // create a new set without the removed holder
+                    final SortedSet<Type> set = new TreeSet<Type>();
+                    set.addAll(byPathSet);
+                    set.remove(holder);
 
-                    // replace the old list with the new one (or remove if 
empty)
-                    if (list.isEmpty()) {
+                    // replace the old set with the new one (or remove if 
empty)
+                    if (set.isEmpty()) {
                         byHostMap.remove(holder.host);
                     } else {
-                        byHostMap.put(holder.host, list);
+                        byHostMap.put(holder.host, set);
                     }
                 }
             }
@@ -100,7 +99,7 @@ public class PathBasedHolderCache<Type e
         }
     }
 
-    public List<Type>[] findApplicableHolder(final HttpServletRequest request) 
{
+    public Collection<Type>[] findApplicableHolder(final HttpServletRequest 
request) {
         this.rwLock.readLock().lock();
         try {
             final String hostname = request.getServerName()
@@ -109,14 +108,14 @@ public class PathBasedHolderCache<Type e
                     : "");
 
             @SuppressWarnings("unchecked")
-            final List<Type>[] result = new ArrayList[4];
+            final SortedSet<Type>[] result = new SortedSet[4];
 
-            final Map<String, List<Type>> byHostMap = 
cache.get(request.getScheme());
+            final Map<String, SortedSet<Type>> byHostMap = 
cache.get(request.getScheme());
             if ( byHostMap != null ) {
                 result[0] = byHostMap.get(hostname);
                 result[1] = byHostMap.get("");
             }
-            final Map<String, List<Type>> defaultByHostMap = cache.get("");
+            final Map<String, SortedSet<Type>> defaultByHostMap = 
cache.get("");
             if ( defaultByHostMap != null ) {
                 result[2] = defaultByHostMap.get(hostname);
                 result[3] = defaultByHostMap.get("");
@@ -131,9 +130,9 @@ public class PathBasedHolderCache<Type e
         this.rwLock.readLock().lock();
         try {
             final List<Type> result = new ArrayList<Type>();
-            for (Map<String, List<Type>> byHostEntry : cache.values()) {
-                for (List<Type> holderList : byHostEntry.values()) {
-                    result.addAll(holderList);
+            for (Map<String, SortedSet<Type>> byHostEntry : cache.values()) {
+                for (SortedSet<Type> holderSet : byHostEntry.values()) {
+                    result.addAll(holderSet);
                 }
             }
             return result;

Modified: 
sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java?rev=1625490&r1=1625489&r2=1625490&view=diff
==============================================================================
--- 
sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
 (original)
+++ 
sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
 Wed Sep 17 08:56:19 2014
@@ -20,6 +20,7 @@ package org.apache.sling.auth.core.impl;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Dictionary;
 import java.util.HashMap;
 import java.util.Hashtable;
@@ -513,14 +514,14 @@ public class SlingAuthenticator implemen
         }
 
         // select path used for authentication handler selection
-        final List<AbstractAuthenticationHandlerHolder>[] holderListArray = 
this.authHandlerCache.findApplicableHolder(request);
+        final Collection<AbstractAuthenticationHandlerHolder>[] holdersArray = 
this.authHandlerCache
+                .findApplicableHolder(request);
         final String path = getHandlerSelectionPath(request);
         boolean done = false;
-        for(int m = 0; !done && m < holderListArray.length; m++) {
-            final List<AbstractAuthenticationHandlerHolder> holderList = 
holderListArray[m];
+        for (int m = 0; !done && m < holdersArray.length; m++) {
+            final Collection<AbstractAuthenticationHandlerHolder> holderList = 
holdersArray[m];
             if ( holderList != null ) {
-                for (int i = 0; !done && i < holderList.size(); i++) {
-                    final AbstractAuthenticationHandlerHolder holder = 
holderList.get(i);
+                for (AbstractAuthenticationHandlerHolder holder : holderList) {
                     if (path.startsWith(holder.path)) {
                         log.debug("login: requesting authentication using 
handler: {}",
                             holder);
@@ -546,9 +547,9 @@ public class SlingAuthenticator implemen
         // no handler could send an authentication request, throw
         if (!done) {
             int size = 0;
-            for (int m = 0; m < holderListArray.length; m++) {
-                if ( holderListArray[m] != null ) {
-                    size += holderListArray[m].size();
+            for (int m = 0; m < holdersArray.length; m++) {
+                if (holdersArray[m] != null) {
+                    size += holdersArray[m].size();
                 }
             }
             log.info("login: No handler for request ({} handlers available)", 
size);
@@ -572,12 +573,12 @@ public class SlingAuthenticator implemen
         setSudoCookie(request, response, new AuthenticationInfo("dummy", 
request.getRemoteUser()));
 
         final String path = getHandlerSelectionPath(request);
-        final List<AbstractAuthenticationHandlerHolder>[] holderListArray = 
this.authHandlerCache.findApplicableHolder(request);
-        for (int m = 0; m < holderListArray.length; m++) {
-            final List<AbstractAuthenticationHandlerHolder> holderList = 
holderListArray[m];
-            if (holderList != null) {
-                for (int i = 0; i < holderList.size(); i++) {
-                    AbstractAuthenticationHandlerHolder holder = 
holderList.get(i);
+        final Collection<AbstractAuthenticationHandlerHolder>[] holdersArray = 
this.authHandlerCache
+                .findApplicableHolder(request);
+        for (int m = 0; m < holdersArray.length; m++) {
+            final Collection<AbstractAuthenticationHandlerHolder> holderSet = 
holdersArray[m];
+            if (holderSet != null) {
+                for (AbstractAuthenticationHandlerHolder holder : holderSet) {
                     if (path.startsWith(holder.path)) {
                         log.debug("logout: dropping authentication using 
handler: {}",
                             holder);
@@ -689,12 +690,12 @@ public class SlingAuthenticator implemen
             path = "/";
         }
 
-        final List<AbstractAuthenticationHandlerHolder>[] localArray = 
this.authHandlerCache.findApplicableHolder(request);
+        final Collection<AbstractAuthenticationHandlerHolder>[] localArray = 
this.authHandlerCache
+                .findApplicableHolder(request);
         for (int m = 0; m < localArray.length; m++) {
-            final List<AbstractAuthenticationHandlerHolder> local = 
localArray[m];
+            final Collection<AbstractAuthenticationHandlerHolder> local = 
localArray[m];
             if (local != null) {
-                for (int i = 0; i < local.size(); i++) {
-                    AbstractAuthenticationHandlerHolder holder = local.get(i);
+                for (AbstractAuthenticationHandlerHolder holder : local) {
                     if (path.startsWith(holder.path)) {
                         final AuthenticationInfo authInfo = 
holder.extractCredentials(
                             request, response);
@@ -882,12 +883,12 @@ public class SlingAuthenticator implemen
             return false;
         }
 
-        final List<AuthenticationRequirementHolder>[] holderListArray = 
authRequiredCache.findApplicableHolder(request);
-        for(int m = 0; m < holderListArray.length; m++) {
-            final List<AuthenticationRequirementHolder> holderList = 
holderListArray[m];
-            if ( holderList != null ) {
-                for (int i = 0; i < holderList.size(); i++) {
-                    final AuthenticationRequirementHolder holder = 
holderList.get(i);
+        final Collection<AuthenticationRequirementHolder>[] holderSetArray = 
authRequiredCache
+                .findApplicableHolder(request);
+        for (int m = 0; m < holderSetArray.length; m++) {
+            final Collection<AuthenticationRequirementHolder> holders = 
holderSetArray[m];
+            if (holders != null) {
+                for (AuthenticationRequirementHolder holder : holders) {
                     if (path.startsWith(holder.path)) {
                         return !holder.requiresAuthentication();
                     }


Reply via email to