You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by pa...@apache.org on 2020/11/30 12:39:53 UTC

[sling-org-apache-sling-servlets-resolver] branch master updated: SLING-9949: speed-up bundled script resolution (#13)

This is an automated email from the ASF dual-hosted git repository.

pauls pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-resolver.git


The following commit(s) were added to refs/heads/master by this push:
     new 945ed2c  SLING-9949: speed-up bundled script resolution (#13)
945ed2c is described below

commit 945ed2c1c540d7486cd70199bab9f38663e6a689
Author: Karl Pauls <pa...@apache.org>
AuthorDate: Mon Nov 30 13:39:46 2020 +0100

    SLING-9949: speed-up bundled script resolution (#13)
    
    * SLING-9949: speed-up bundled script resolution
---
 .../tracker/internal/BundledScriptServlet.java     | 15 +++++-----
 .../tracker/internal/BundledScriptTracker.java     | 33 +++++++++++-----------
 .../resource/MergingServletResourceProvider.java   | 28 ++++++------------
 3 files changed, 33 insertions(+), 43 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..f8102b1 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,22 +34,23 @@ 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;
 
 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,
                          @NotNull BundledRenderUnit executable) {
-        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 +74,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..2831a23 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 = cache.computeIfAbsent(cap, BundledRenderUnitCapabilityImpl::fromBundleCapability);
                     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 = cache.computeIfAbsent(wire.getCapability(), BundledRenderUnitCapabilityImpl::fromBundleCapability);
             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 = cache.computeIfAbsent(wire.getCapability(), BundledRenderUnitCapabilityImpl::fromBundleCapability);
             if (wiredCapability.getSelectors().isEmpty()) {
                 Bundle providingBundle = wire.getProvider().getBundle();
                 requiresChain.add(new TypeProviderImpl(wiredCapability, providingBundle));
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);
                     }
                 }
             }