Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1549717 - in
 /tomcat/trunk/java/org/apache/catalina/webresources: Cache.java
 CachedResource.java
Date: Tue, 10 Dec 2013 00:16:45 -0000
To: dev@tomcat.apache.org
From: markt@apache.org
Message-Id: <20131210001645.DFA8523888A6@eris.apache.org>

Author: markt
Date: Tue Dec 10 00:16:45 2013
New Revision: 1549717

URL: http://svn.apache.org/r1549717
Log:
Change the caching approach slightly.
Previously, resources were only cached if they were smaller than maxObjectSizeBytes. Now they are always cached (so the metadata is cached) but the content is only cached if they are smaller than maxObjectSizeBytes.

Modified:
    tomcat/trunk/java/org/apache/catalina/webresources/Cache.java
    tomcat/trunk/java/org/apache/catalina/webresources/CachedResource.java

Modified: tomcat/trunk/java/org/apache/catalina/webresources/Cache.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/Cache.java?rev=1549717&r1=1549716&r2=1549717&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/webresources/Cache.java (original)
+++ tomcat/trunk/java/org/apache/catalina/webresources/Cache.java Tue Dec 10 00:16:45 2013
@@ -34,10 +34,6 @@ public class Cache {
     protected static final StringManager sm =
             StringManager.getManager(Constants.Package);
 
-    // Estimate (on high side to be safe) of average size excluding content
-    // based on profiler data.
-    private static final long CACHE_ENTRY_SIZE = 500;
-
     private static final long TARGET_FREE_PERCENT_GET = 5;
     private static final long TARGET_FREE_PERCENT_BACKGROUND = 10;
 
@@ -70,7 +66,11 @@ public class Cache {
         }
 
         if (cacheEntry == null) {
-            CachedResource newCacheEntry = new CachedResource(root, path, ttl);
+            // Local copy to ensure consistency
+            int maxObjectSizeBytes = getMaxObjectSizeBytes();
+            CachedResource newCacheEntry =
+                    new CachedResource(root, path, getTtl(), maxObjectSizeBytes);
+
             // Concurrent callers will end up with the same CachedResource
             // instance
             cacheEntry = resourceCache.putIfAbsent(path, newCacheEntry);
@@ -79,19 +79,11 @@ public class Cache {
                 // newCacheEntry was inserted into the cache - validate it
                 cacheEntry = newCacheEntry;
                 cacheEntry.validate(useClassLoaderResources);
-                if (cacheEntry.getContentLength() > getMaxObjectSizeBytes()) {
-                    // Cache size has not been updated at this point
-                    removeCacheEntry(path, false);
-                    // Return the original resource not the one wrapped in the
-                    // cache otherwise content will be cached any way.
-                    return cacheEntry.getWebResource();
-                }
 
-                // Assume that the cache entry will include the content.
-                // This isn't always the case but it makes tracking the
-                // current cache size easier.
-                long delta = CACHE_ENTRY_SIZE;
-                delta += cacheEntry.getContentLength();
+                // Even if the resource content larger than maxObjectSizeBytes
+                // there is still benefit in caching the resource metadata
+
+                long delta = cacheEntry.getSize();
                 size.addAndGet(delta);
 
                 if (size.get() > maxSize) {
@@ -181,9 +173,8 @@ public class Cache {
         // once and the cache size is only updated (if required) once.
         CachedResource cachedResource = resourceCache.remove(path);
         if (cachedResource != null && updateSize) {
-            long delta =
-                    0 - CACHE_ENTRY_SIZE - cachedResource.getContentLength();
-            size.addAndGet(delta);
+            long delta = cachedResource.getSize();
+            size.addAndGet(-delta);
         }
     }
 
@@ -221,8 +212,7 @@ public class Cache {
         return maxObjectSize / 1024;
     }
 
-    public long getMaxObjectSizeBytes() {
-        // Internally bytes, externally kilobytes
+    public int getMaxObjectSizeBytes() {
         return maxObjectSize;
     }
 

Modified: tomcat/trunk/java/org/apache/catalina/webresources/CachedResource.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/CachedResource.java?rev=1549717&r1=1549716&r2=1549717&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/webresources/CachedResource.java (original)
+++ tomcat/trunk/java/org/apache/catalina/webresources/CachedResource.java Tue Dec 10 00:16:45 2013
@@ -32,9 +32,14 @@ import org.apache.catalina.WebResourceRo
  */
 public class CachedResource implements WebResource {
 
+    // Estimate (on high side to be safe) of average size excluding content
+    // based on profiler data.
+    private static final long CACHE_ENTRY_SIZE = 500;
+
     private final StandardRoot root;
     private final String webAppPath;
     private final long ttl;
+    private final int maxObjectSizeBytes;
 
     private volatile WebResource webResource;
     private volatile long nextCheck;
@@ -49,10 +54,12 @@ public class CachedResource implements W
     private volatile Long cachedContentLength = null;
 
 
-    public CachedResource(StandardRoot root, String path, long ttl) {
+    public CachedResource(StandardRoot root, String path, long ttl,
+            int maxObjectSizeBytes) {
         this.root = root;
         this.webAppPath = path;
         this.ttl = ttl;
+        this.maxObjectSizeBytes = maxObjectSizeBytes;
     }
 
     protected boolean validate(boolean useClassLoaderResources) {
@@ -232,6 +239,9 @@ public class CachedResource implements W
     public byte[] getContent() {
         byte[] cachedContent = this.cachedContent;
         if (cachedContent == null) {
+            if (getContentLength() > maxObjectSizeBytes) {
+                return null;
+            }
             cachedContent = webResource.getContent();
             this.cachedContent = cachedContent;
         }
@@ -266,4 +276,15 @@ public class CachedResource implements W
     WebResource getWebResource() {
         return webResource;
     }
+
+    // Assume that the cache entry will always include the content unless the
+    // resource content is larger than maxObjectSizeBytes. This isn't always the
+    // case but it makes tracking the current cache size easier.
+    long getSize() {
+        long result = CACHE_ENTRY_SIZE;
+        if (getContentLength() <= maxObjectSizeBytes) {
+            result += getContentLength();
+        }
+        return result;
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org