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