[TAP5-2461] Make proper use of cache when finding localized Resources and Assets

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 5.4
• Component/s: tapestry-core, tapestry-ioc
• Labels: patch performance
• Assignee: Howard Lewis Ship
• Reporter: Michael Mikhulya
• Created: 05/Mar/15 14:45
• Updated: 02/Jun/15 00:35
• Resolved: 01/Jun/15 23:31

Description

Following locks are very very frequent on production environment.

"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)

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:

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);
...

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.