joerghoh commented on code in PR #26:
URL:
https://github.com/apache/sling-org-apache-sling-scripting-sightly/pull/26#discussion_r1665858945
##########
src/main/java/org/apache/sling/scripting/sightly/impl/utils/ScriptDependencyResolver.java:
##########
@@ -154,24 +196,6 @@ public void bundleChanged(BundleEvent event) {
}
}
- private Resource getResource(@NotNull ResourceResolver resolver, @NotNull
Resource resource) {
Review Comment:
Just for confirmation: You removed this method (or replaced it in the
``internalResolveScript`` with a ``Resolver.getResource()`` because
``resource.getPath()`` will always return an absolute path, and the the
``else`` path would never be invoked at all?
##########
src/main/java/org/apache/sling/scripting/sightly/impl/utils/ScriptDependencyResolver.java:
##########
@@ -74,60 +76,100 @@ public class ScriptDependencyResolver implements
ResourceChangeListener, Externa
private ResourceResolverFactory resourceResolverFactory;
private Map<String, String> resolutionCache = new Cache(0);
- private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
+ private final ReentrantReadWriteLock rwl = new
ReentrantReadWriteLock(true);
private final Lock readLock = rwl.readLock();
private final Lock writeLock = rwl.writeLock();
+ private ServiceRegistration<ResourceChangeListener>
resourceChangeListenerServiceRegistration;
+
+ private boolean cacheEnabled = false;
+
+ private static final String NOT_FOUND_MARKER = "#NOT_FOUND#";
+
+ @SuppressWarnings("squid:S1149")
@Activate
private void activate(ComponentContext componentContext) {
int cacheSize =
sightlyEngineConfiguration.getScriptResolutionCacheSize();
if (cacheSize < 1024) {
+ cacheEnabled = false;
resolutionCache = new Cache(0);
} else {
+ cacheEnabled = true;
resolutionCache = new Cache(cacheSize);
}
- componentContext.getBundleContext().addBundleListener(this);
+ if (cacheEnabled) {
+ componentContext.getBundleContext().addBundleListener(this);
+ Dictionary<String, Object> resourceChangeListenerProperties = new
Hashtable<>();
+ resourceChangeListenerProperties.put(ResourceChangeListener.PATHS,
".");
+
resourceChangeListenerProperties.put(ResourceChangeListener.CHANGES, new
String[] {
+ ResourceChangeListener.CHANGE_ADDED,
+ ResourceChangeListener.CHANGE_CHANGED,
+ ResourceChangeListener.CHANGE_REMOVED
+ });
+ resourceChangeListenerServiceRegistration =
+
componentContext.getBundleContext().registerService(ResourceChangeListener.class,
this, resourceChangeListenerProperties);
+ }
+ }
+
+ @Deactivate
+ private void deactivate(ComponentContext componentContext) {
+ if (resourceChangeListenerServiceRegistration != null) {
+ resourceChangeListenerServiceRegistration.unregister();
+ }
+ componentContext.getBundleContext().removeBundleListener(this);
}
public Resource resolveScript(RenderContext renderContext, String
scriptIdentifier) {
+ SlingHttpServletRequest request =
BindingsUtils.getRequest(renderContext.getBindings());
+ if (!cacheEnabled) {
+ return internalResolveScript(request, renderContext,
scriptIdentifier);
+ }
readLock.lock();
try {
- SlingHttpServletRequest request =
BindingsUtils.getRequest(renderContext.getBindings());
String cacheKey = request.getResource().getResourceType() + ":" +
scriptIdentifier;
Resource result = null;
if (!resolutionCache.containsKey(cacheKey)) {
readLock.unlock();
writeLock.lock();
try {
- Resource caller =
-
ResourceResolution.getResourceForRequest(scriptingResourceResolverProvider.getRequestScopedResourceResolver(),
- request);
- result =
ResourceResolution.getResourceFromSearchPath(caller, scriptIdentifier);
- if (result == null) {
- SlingScriptHelper sling =
BindingsUtils.getHelper(renderContext.getBindings());
- if (sling != null) {
- caller =
getResource(scriptingResourceResolverProvider.getRequestScopedResourceResolver(),
- sling.getScript().getScriptResource());
- result =
ResourceResolution.getResourceFromSearchPath(caller, scriptIdentifier);
- }
- }
+ result = internalResolveScript(request, renderContext,
scriptIdentifier);
Review Comment:
this call is done with the writeLock held, so it's serialized. Would it be
possible to run this code outside of the writeLock and do only the update of
the resolutionCache with the writeLock held?
--
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]
