[ 
https://issues.apache.org/jira/browse/SLING-12344?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17862750#comment-17862750
 ] 

Carsten Ziegeler edited comment on SLING-12344 at 7/3/24 12:05 PM:
-------------------------------------------------------------------

I think this code violates one of the core principles of Sling's request 
processing - normal request processing should avoid blocks where all requests 
need to be synchronized. Granted, this is an unwritten principle - but we tried 
hard to avoid such scenarios.
Here, it seems if the cache is too small or turned off (size of 0) all requests 
go through the write lock


was (Author: cziegeler):
I think this code violates one of the core principles of Sling's request 
processing - normal request processing should avoid blocks very all requests 
need to be synchronized. Granted, this is an unwritten principle - but we tried 
hard to avoid such scenarios.
Here, it seems if the cache is too small or turned off (size of 0) all requests 
go through the write lock

> Lock contention in ScriptDependencyResolver
> -------------------------------------------
>
>                 Key: SLING-12344
>                 URL: https://issues.apache.org/jira/browse/SLING-12344
>             Project: Sling
>          Issue Type: Task
>          Components: HTL
>    Affects Versions: Scripting HTL Engine 1.4.22-1.4.0
>            Reporter: Joerg Hoh
>            Assignee: Radu Cotescu
>            Priority: Major
>
> I see threaddumps which show lock contention in the ScriptDependencyResolver 
> like this:
> {noformat}
>         at jdk.internal.misc.Unsafe.park([email protected]/Native Method)
>         - parking to wait for  <0x0000000496e16af0> (a 
> java.util.concurrent.locks.ReentrantReadWriteLock$NonfairSync)
>         at 
> java.util.concurrent.locks.LockSupport.park([email protected]/LockSupport.java:194)
>         at 
> java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt([email protected]/AbstractQueuedSynchronizer.java:885)
>         at 
> java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireQueued([email protected]/AbstractQueuedSynchronizer.java:917)
>         at 
> java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire([email protected]/AbstractQueuedSynchronizer.java:1240)
>         at 
> java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock.lock([email protected]/ReentrantReadWriteLock.java:959)
>         at 
> org.apache.sling.scripting.sightly.impl.utils.ScriptDependencyResolver.resolveScript(ScriptDependencyResolver.java:100)
>         at 
> org.apache.sling.scripting.sightly.impl.engine.extension.use.RenderUnitProvider.provide(RenderUnitProvider.java:95)
>         at 
> org.apache.sling.scripting.sightly.impl.engine.extension.use.UseRuntimeExtension.call(UseRuntimeExtension.java:71)
>         at 
> org.apache.sling.scripting.sightly.impl.engine.runtime.RenderContextImpl.call(RenderContextImpl.java:72)
>         at apps.components.x.y.y__002e__html.render(y__002e__html.java:39)
> {noformat}
> but also:
> {noformat}
>         at jdk.internal.misc.Unsafe.park([email protected]/Native Method)
>         - parking to wait for  <0x0000000496e16af0> (a 
> java.util.concurrent.locks.ReentrantReadWriteLock$NonfairSync)
>         at 
> java.util.concurrent.locks.LockSupport.park([email protected]/LockSupport.java:194)
>         at 
> java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt([email protected]/AbstractQueuedSynchronizer.java:885)
>         at 
> java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireShared([email protected]/AbstractQueuedSynchronizer.java:1009)
>         at 
> java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireShared([email protected]/AbstractQueuedSynchronizer.java:1324)
>         at 
> java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock.lock([email protected]/ReentrantReadWriteLock.java:738)
>         at 
> org.apache.sling.scripting.sightly.impl.utils.ScriptDependencyResolver.resolveScript(ScriptDependencyResolver.java:93)
>         at 
> org.apache.sling.scripting.sightly.impl.engine.extension.use.RenderUnitProvider.provide(RenderUnitProvider.java:95)
>         at 
> org.apache.sling.scripting.sightly.impl.engine.extension.use.UseRuntimeExtension.call(UseRuntimeExtension.java:71)
>         at 
> org.apache.sling.scripting.sightly.impl.engine.runtime.RenderContextImpl.call(RenderContextImpl.java:72)
>         at apps.components.a.b.b__002e__html$3.render(b__002e__html.java:218)
> {noformat}
> I see that the ScriptDependencyResolver holds cache, which just saves 
> positive results where the result has been found; if the result is null, it's 
> not cached, and it will be attempted over and over again.
> Of course this situation mostly happens if a lot of these requests with 
> invalid scriptIdentifiers are done, which points to problems in the content. 
> But it would be great if the code behaves a bit better here, because I have a 
> situation here where requests holding the write lock block all other requests 
> which would have a cache hit.
> (Thinking further, I don't really understand why a ReadWrite lock is used 
> here at all. As far as I can see a ConcurrentHashMap as a cache should be 
> sufficient.)



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to