Bug 57752

Summary: WebResources Cache 'lookupCount' counts non-lookups
Product: Tomcat 8 Reporter: Adam Mlodzinski <adam.mlodzinski>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal CC: adam.mlodzinski
Priority: P2    
Version: 8.0.20   
Target Milestone: ----   
Hardware: PC   
OS: All   

Description Adam Mlodzinski 2015-03-25 00:29:43 UTC
The method org.apache.catalina.webresources.Cache.getResource(String, boolean) is used to retrieve a resource given a particular path.  Some resources are cached by the Cache class, while others bypass the cache.  A request for those resources which bypass the cache are forwarded somewhere else to find the resource (the 'root' object).
The problem is that every request for a resource, whether for a resource that is potentially in the cache, or for a resource which always bypasses the cache, is counted as a resource lookup, while the hit count is incremented only if the resource was found in the cache.  This leads to a highly disproportionate ratio of lookups to hits from the cache.

There are 2 ways to fix this:

1)  If a resource request bypasses the cache, do not increment the lookup count.

OR

2) If a resource request which bypasses the cache was found by the alternate source, then increment the hit count.

I think the 2nd option would lead to values higher than they ought to be.  The first option ought to provide more accurate representation of the usage of the cache.


For the fist option, the code fix is trivial:
{code}
*** Cache.java  2015-02-15 13:16:26.000000000 -0500
--- Cache.java.fixed    2015-03-24 19:08:52.630609500 -0400
***************
*** 59,70 ****

      protected WebResource getResource(String path, boolean useClassLoaderResources) {

-         lookupCount.incrementAndGet();
-
          if (noCache(path)) {
              return root.getResourceInternal(path, useClassLoaderResources);
          }

          CachedResource cacheEntry = resourceCache.get(path);

          if (cacheEntry != null && !cacheEntry.validateResource(useClassLoaderResources)) {
--- 59,70 ----

      protected WebResource getResource(String path, boolean useClassLoaderResources) {

          if (noCache(path)) {
              return root.getResourceInternal(path, useClassLoaderResources);
          }

+         lookupCount.incrementAndGet();
+
          CachedResource cacheEntry = resourceCache.get(path);

          if (cacheEntry != null && !cacheEntry.validateResource(useClassLoaderResources)) {
{code}
Comment 1 Mark Thomas 2015-03-26 21:51:20 UTC
Thanks for the report, the spot-on analysis and the patch. I agree the first option is beter.

Your patch has been applied to trunk and to 8.0.x for 8.0.22 onwards.

Thanks again.