[jira] [Commented] (TAP5-2461) Lock contention on ZipFile.getEntry

["Thiago H. de Paula Figueiredo (JIRA)" <ji...@apache.org>]

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

Thiago H. de Paula Figueiredo commented on TAP5-2461:
-----------------------------------------------------

Hello, [~mihasik]! I hope you enjoyed your vacation. :-)

Good catch! How did it work before 5.3.4? How the code differs? If the pre-5.3.4 performance is good, without this lock contention you've found, I believe we should use it unless there's some good reason not to (such as why this code was changed in first place!

Regarding using a Map to build a cache, I'm afraid this can create a situation in which less-used objects enter it and never leave, causing excessive memory usage. I'd use WeakHashMap instead, as it will release some objects when free memory is low.

Cheers!

> Lock contention on ZipFile.getEntry
> -----------------------------------
>
>                 Key: TAP5-2461
>                 URL: https://issues.apache.org/jira/browse/TAP5-2461
>             Project: Tapestry 5
>          Issue Type: Bug
>          Components: tapestry-core, tapestry-ioc
>            Reporter: Michael Mikhulya
>              Labels: performance
>
> Following locks are very very frequent on production environment.
> {noformat}
> "catalina-exec-244" #9143 daemon prio=5 os_prio=0 tid=0x00007fb350209800 nid=0x4b0a waiting for monitor entry [0x00007fb1a76b3000]
>    java.lang.Thread.State: BLOCKED (on object monitor)
>         at java.util.zip.ZipFile.getEntry(ZipFile.java:309)
>         - locked <0x00000006490a34a0> (a java.util.jar.JarFile)
>         at java.util.jar.JarFile.getEntry(JarFile.java:240)
>         at java.util.jar.JarFile.getJarEntry(JarFile.java:223)
>         at sun.misc.URLClassPath$JarLoader.getResource(URLClassPath.java:841)
>         at sun.misc.URLClassPath$JarLoader.findResource(URLClassPath.java:819)
>         at sun.misc.URLClassPath.findResource(URLClassPath.java:176)
>         at java.net.URLClassLoader$2.run(URLClassLoader.java:557)
>         at java.net.URLClassLoader$2.run(URLClassLoader.java:555)
>         at java.security.AccessController.doPrivileged(Native Method)
>         at java.net.URLClassLoader.findResource(URLClassLoader.java:554)
>         at java.lang.ClassLoader.getResource(ClassLoader.java:1093)
>         at org.apache.catalina.loader.WebappClassLoaderBase.getResource(WebappClassLoaderBase.java:1042)
>         at org.apache.tapestry5.ioc.internal.util.ClasspathResource.resolveURL(ClasspathResource.java:79)
>         at org.apache.tapestry5.ioc.internal.util.ClasspathResource.toURL(ClasspathResource.java:61)
>         at org.apache.tapestry5.ioc.internal.util.AbstractResource.computeExists(AbstractResource.java:236)
>         at org.apache.tapestry5.ioc.internal.util.AbstractResource.exists(AbstractResource.java:218)
>         at org.apache.tapestry5.ioc.internal.util.AbstractResource.findLocalizedResource(AbstractResource.java:177)
>         at org.apache.tapestry5.ioc.internal.util.AbstractResource.populateLocalizationCache(AbstractResource.java:159)
>         at org.apache.tapestry5.ioc.internal.util.AbstractResource.forLocale(AbstractResource.java:136)
>         at org.apache.tapestry5.internal.services.AssetSourceImpl.getLocalizedAssetFromResource(AssetSourceImpl.java:134)
>         at org.apache.tapestry5.internal.services.AssetSourceImpl.getAssetInLocale(AssetSourceImpl.java:107)
>         at org.apache.tapestry5.internal.services.AssetSourceImpl.getAsset(AssetSourceImpl.java:87)
>         at $AssetSource_ef3168bbfb42e.getAsset(Unknown Source)
>         at org.apache.tapestry5.internal.services.javascript.CoreJavaScriptStack.getJavaScriptLibraries(CoreJavaScriptStack.java:154)
> {noformat}
> I investegated a reason of issue.
> Since tapestry-5.3.4 {{AbstractResource.forLocale}} returns the cached result.
> The problem is that cache is not used in this case.
> For example:
> {code:title=AssetSourceImpl.java}
>     private Resource findResource(Resource baseResource, String path)
>     {
>         assert path != null;
>         int colonx = path.indexOf(':');
>         if (colonx < 0)
>         {
>             Resource root = baseResource != null ? baseResource : prefixToRootResource.get(AssetConstants.CLASSPATH);
>             return root.forFile(path);
> ...
> {code}
> Here each time new resource is created with empty cache.
> So when we call {{AbstractResource.forLocale}} it is forced to call {{AbstractResource.findLocalizedResource}}.
> I suggest to add simple caches (ConcurrentHashMap) in {{ClasspathResource}} and {{ContextResource}} and use this cache inside {{newResource}} method.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)