This is an automated email from the ASF dual-hosted git repository. pauls pushed a commit to branch issues/SLING-9949 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-resolver.git
commit 35e2384782aa030a4bb9e604d1c749d4ddee95d2 Author: Karl Pauls <[email protected]> AuthorDate: Mon Nov 30 11:37:11 2020 +0100 SLING-9949: speed-up bundled script resolution --- .../tracker/internal/BundledScriptServlet.java | 12 ++++--- .../tracker/internal/BundledScriptTracker.java | 42 +++++++++++++--------- .../resource/MergingServletResourceProvider.java | 28 +++++---------- 3 files changed, 42 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/apache/sling/servlets/resolver/bundle/tracker/internal/BundledScriptServlet.java b/src/main/java/org/apache/sling/servlets/resolver/bundle/tracker/internal/BundledScriptServlet.java index ad5bb16..6e59a58 100644 --- a/src/main/java/org/apache/sling/servlets/resolver/bundle/tracker/internal/BundledScriptServlet.java +++ b/src/main/java/org/apache/sling/servlets/resolver/bundle/tracker/internal/BundledScriptServlet.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.Collection; import java.util.LinkedHashSet; +import java.util.Set; import java.util.stream.Collectors; import javax.servlet.GenericServlet; @@ -33,6 +34,7 @@ import org.apache.sling.api.SlingConstants; import org.apache.sling.api.SlingHttpServletRequest; import org.apache.sling.api.SlingHttpServletResponse; import org.apache.sling.servlets.resolver.bundle.tracker.BundledRenderUnit; +import org.apache.sling.servlets.resolver.bundle.tracker.ResourceType; import org.apache.sling.servlets.resolver.bundle.tracker.TypeProvider; import org.apache.sling.servlets.resolver.bundle.tracker.internal.request.RequestWrapper; import org.jetbrains.annotations.NotNull; @@ -42,6 +44,7 @@ public class BundledScriptServlet extends GenericServlet { private final LinkedHashSet<TypeProvider> wiredTypeProviders; private final BundledRenderUnit executable; private final String servletInfo; + private final Set<ResourceType> types; BundledScriptServlet(@NotNull LinkedHashSet<TypeProvider> wiredTypeProviders, @@ -49,6 +52,8 @@ public class BundledScriptServlet extends GenericServlet { this.wiredTypeProviders = wiredTypeProviders; this.executable = executable; this.servletInfo = "Script " + executable.getPath(); + this.types = wiredTypeProviders.stream().map(typeProvider -> typeProvider.getBundledRenderUnitCapability().getResourceTypes() + ).flatMap(Collection::stream).collect(Collectors.toSet()); } @Override @@ -72,14 +77,13 @@ public class BundledScriptServlet extends GenericServlet { } } - RequestWrapper requestWrapper = new RequestWrapper(request, - wiredTypeProviders.stream().map(typeProvider -> typeProvider.getBundledRenderUnitCapability().getResourceTypes() - ).flatMap(Collection::stream).collect(Collectors.toSet())); + RequestWrapper requestWrapper = new RequestWrapper(request, types); try { executable.eval(requestWrapper, response); } catch (Exception se) { Throwable cause = (se.getCause() == null) ? se : se.getCause(); - throw new ServletException(String.format("Failed executing script %s: %s", executable.getPath(), se.getMessage()), cause); + throw cause instanceof ServletException ? (ServletException) cause : + new ServletException(String.format("Failed executing script %s: %s", executable.getPath(), se.getMessage()), cause); } } else { throw new ServletException("Not a Sling HTTP request/response"); diff --git a/src/main/java/org/apache/sling/servlets/resolver/bundle/tracker/internal/BundledScriptTracker.java b/src/main/java/org/apache/sling/servlets/resolver/bundle/tracker/internal/BundledScriptTracker.java index a37cece..a0febde 100644 --- a/src/main/java/org/apache/sling/servlets/resolver/bundle/tracker/internal/BundledScriptTracker.java +++ b/src/main/java/org/apache/sling/servlets/resolver/bundle/tracker/internal/BundledScriptTracker.java @@ -133,13 +133,14 @@ public class BundledScriptTracker implements BundleTrackerCustomizer<List<Servic .anyMatch(m_context.getBundle()::equals)) { LOGGER.debug("Inspecting bundle {} for {} capability.", bundle.getSymbolicName(), NS_SLING_SERVLET); List<BundleCapability> capabilities = bundleWiring.getCapabilities(NS_SLING_SERVLET); - Set<TypeProvider> requiresChain = collectRequiresChain(bundleWiring); + Map<BundleCapability, BundledRenderUnitCapability> cache = new HashMap<>(); + Set<TypeProvider> requiresChain = collectRequiresChain(bundleWiring, cache); if (!capabilities.isEmpty()) { List<ServiceRegistration<Servlet>> serviceRegistrations = capabilities.stream().flatMap(cap -> { Hashtable<String, Object> properties = new Hashtable<>(); properties.put(Constants.SERVICE_DESCRIPTION, BundledScriptServlet.class.getName() + cap.getAttributes()); - BundledRenderUnitCapability bundledRenderUnitCapability = BundledRenderUnitCapabilityImpl.fromBundleCapability(cap); + BundledRenderUnitCapability bundledRenderUnitCapability = cachedOrNew(cap,cache); BundledRenderUnit executable = null; TypeProvider baseTypeProvider = new TypeProviderImpl(bundledRenderUnitCapability, bundle); LinkedHashSet<TypeProvider> inheritanceChain = new LinkedHashSet<>(); @@ -167,7 +168,7 @@ public class BundledScriptTracker implements BundleTrackerCustomizer<List<Servic String extendedResourceTypeString = bundledRenderUnitCapability.getExtendedResourceType(); if (StringUtils.isNotEmpty(extendedResourceTypeString)) { - collectInheritanceChain(inheritanceChain, bundleWiring, extendedResourceTypeString); + collectInheritanceChain(inheritanceChain, bundleWiring, extendedResourceTypeString, cache); inheritanceChain.stream().filter(typeProvider -> typeProvider.getBundledRenderUnitCapability().getResourceTypes().stream() .anyMatch(resourceType -> resourceType.getType().equals(extendedResourceTypeString))).findFirst() .ifPresent(typeProvider -> { @@ -190,10 +191,10 @@ public class BundledScriptTracker implements BundleTrackerCustomizer<List<Servic List<ServiceRegistration<Servlet>> regs = new ArrayList<>(); if (executable != null) { - BundledRenderUnit finalExecutable = executable; - final String executableParentPath = ResourceUtil.getParent(executable.getPath()); - if (executable.getPath().equals(bundledRenderUnitCapability.getPath())) { - properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, executable.getPath()); + String executablePath = executable.getPath(); + final String executableParentPath = ResourceUtil.getParent(executablePath); + if (executablePath.equals(bundledRenderUnitCapability.getPath())) { + properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, executablePath); } else { if (!bundledRenderUnitCapability.getResourceTypes().isEmpty() && bundledRenderUnitCapability.getSelectors().isEmpty() && StringUtils.isEmpty(bundledRenderUnitCapability.getExtension()) && @@ -214,7 +215,7 @@ public class BundledScriptTracker implements BundleTrackerCustomizer<List<Servic }); if (noMatch) { List<String> paths = new ArrayList<>(); - paths.add(finalExecutable.getPath()); + paths.add(executablePath); bundledRenderUnitCapability.getResourceTypes().forEach(resourceType -> { String resourceTypePath = resourceType.toString(); String label; @@ -234,15 +235,15 @@ public class BundledScriptTracker implements BundleTrackerCustomizer<List<Servic if (!properties.containsKey(ServletResolverConstants.SLING_SERVLET_PATHS)) { bundledRenderUnitCapability.getResourceTypes().forEach(resourceType -> { if (StringUtils.isNotEmpty(executableParentPath) && (executableParentPath + "/").startsWith(resourceType.toString() + "/")) { - properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, finalExecutable.getPath()); + properties.put(ServletResolverConstants.SLING_SERVLET_PATHS, executablePath); } }); } } properties.put(ServletResolverConstants.SLING_SERVLET_NAME, - String.format("%s (%s)", BundledScriptServlet.class.getSimpleName(), executable.getPath())); + String.format("%s (%s)", BundledScriptServlet.class.getSimpleName(), executablePath)); regs.add( - register(bundle.getBundleContext(), new BundledScriptServlet(inheritanceChain, executable),properties) + register(bundle.getBundleContext(), new BundledScriptServlet(inheritanceChain, executable), properties) ); } else { LOGGER.warn(String.format("Unable to locate an executable for capability %s.", cap)); @@ -559,9 +560,9 @@ public class BundledScriptTracker implements BundleTrackerCustomizer<List<Servic } private void collectInheritanceChain(@NotNull Set<TypeProvider> providers, @NotNull BundleWiring wiring, - @NotNull String extendedResourceType) { + @NotNull String extendedResourceType, @NotNull Map<BundleCapability, BundledRenderUnitCapability> cache) { for (BundleWire wire : wiring.getRequiredWires(NS_SLING_SERVLET)) { - BundledRenderUnitCapability wiredCapability = BundledRenderUnitCapabilityImpl.fromBundleCapability(wire.getCapability()); + BundledRenderUnitCapability wiredCapability = cachedOrNew(wire.getCapability(), cache); if (wiredCapability.getSelectors().isEmpty()) { for (ResourceType resourceType : wiredCapability.getResourceTypes()) { if (extendedResourceType.equals(resourceType.getType())) { @@ -569,7 +570,7 @@ public class BundledScriptTracker implements BundleTrackerCustomizer<List<Servic providers.add(new TypeProviderImpl(wiredCapability, providingBundle)); String wiredExtends = wiredCapability.getExtendedResourceType(); if (StringUtils.isNotEmpty(wiredExtends)) { - collectInheritanceChain(providers, wire.getProviderWiring(), wiredExtends); + collectInheritanceChain(providers, wire.getProviderWiring(), wiredExtends, cache); } } } @@ -577,10 +578,10 @@ public class BundledScriptTracker implements BundleTrackerCustomizer<List<Servic } } - private Set<TypeProvider> collectRequiresChain(@NotNull BundleWiring wiring) { + private Set<TypeProvider> collectRequiresChain(@NotNull BundleWiring wiring, Map<BundleCapability, BundledRenderUnitCapability> cache) { Set<TypeProvider> requiresChain = new LinkedHashSet<>(); for (BundleWire wire : wiring.getRequiredWires(NS_SLING_SERVLET)) { - BundledRenderUnitCapability wiredCapability = BundledRenderUnitCapabilityImpl.fromBundleCapability(wire.getCapability()); + BundledRenderUnitCapability wiredCapability = cachedOrNew(wire.getCapability(), cache); if (wiredCapability.getSelectors().isEmpty()) { Bundle providingBundle = wire.getProvider().getBundle(); requiresChain.add(new TypeProviderImpl(wiredCapability, providingBundle)); @@ -588,4 +589,13 @@ public class BundledScriptTracker implements BundleTrackerCustomizer<List<Servic } return requiresChain; } + + private BundledRenderUnitCapability cachedOrNew(BundleCapability capability, Map<BundleCapability, BundledRenderUnitCapability> cache) { + BundledRenderUnitCapability result = cache.get(capability); + if (result == null) { + result = BundledRenderUnitCapabilityImpl.fromBundleCapability(capability); + cache.put(capability, result); + } + return result; + } } diff --git a/src/main/java/org/apache/sling/servlets/resolver/internal/resource/MergingServletResourceProvider.java b/src/main/java/org/apache/sling/servlets/resolver/internal/resource/MergingServletResourceProvider.java index d1d23c1..df8ccb4 100644 --- a/src/main/java/org/apache/sling/servlets/resolver/internal/resource/MergingServletResourceProvider.java +++ b/src/main/java/org/apache/sling/servlets/resolver/internal/resource/MergingServletResourceProvider.java @@ -47,11 +47,9 @@ public class MergingServletResourceProvider { synchronized void add(ServletResourceProvider provider, ServiceReference<?> reference) { registrations.add(Pair.of(provider, reference)); - ConcurrentHashMap<String, Set<String>> localTree = localTree(); - ConcurrentHashMap<String, Pair<ServletResourceProvider, ServiceReference<?>>> localProvs = localProviders(); + ConcurrentHashMap<String, Set<String>> localTree = tree.get(); + ConcurrentHashMap<String, Pair<ServletResourceProvider, ServiceReference<?>>> localProvs = providers.get(); index(localTree, localProvs, Arrays.asList(registrations.get(registrations.size() - 1))); - tree.set(localTree); - providers.set(localProvs); } synchronized boolean remove(ServletResourceProvider provider, ServiceReference<?> reference) { @@ -86,18 +84,6 @@ public class MergingServletResourceProvider { providers.set(new ConcurrentHashMap<>()); } - private ConcurrentHashMap<String, Set<String>> localTree() { - ConcurrentHashMap<String, Set<String>> localTree = new ConcurrentHashMap<>(); - for (Map.Entry<String, Set<String>> entry : tree.get().entrySet()) { - localTree.put(entry.getKey(), Collections.synchronizedSet(new LinkedHashSet<>(entry.getValue()))); - } - return localTree; - } - - private ConcurrentHashMap<String, Pair<ServletResourceProvider, ServiceReference<?>>> localProviders() { - return new ConcurrentHashMap<>(providers.get()); - } - private void index(ConcurrentHashMap<String, Set<String>> tree, ConcurrentHashMap<String, Pair<ServletResourceProvider, ServiceReference<?>>> providers, List<Pair<ServletResourceProvider, ServiceReference<?>>> registrations) { for (Pair<ServletResourceProvider, ServiceReference<?>> reference : registrations) { for (String path : reference.getLeft().getServletPaths()) { @@ -114,10 +100,12 @@ public class MergingServletResourceProvider { childs.add(current); } - Pair<ServletResourceProvider, ServiceReference<?>> old = providers.put(path, reference); - if (old != null) { - if (reference.getRight().compareTo(old.getRight()) < 0) { - providers.put(path, old); + Pair<ServletResourceProvider, ServiceReference<?>> old = providers.get(path); + if (old == null) { + providers.put(path, reference); + } else { + if (reference.getRight().compareTo(old.getRight()) > 0) { + providers.put(path, reference); } } }
