Author: mck
Date: Thu Aug 2 12:32:27 2012
New Revision: 1368433
URL: http://svn.apache.org/viewvc?rev=1368433&view=rev
Log:
TILES-557 - concurrency failures in OptionsRenderer.Cache
replace ConcurrentHashMap with Guava's CacheBuilder. cleanup usage and docs
around Cache.
Modified:
tiles/framework/trunk/tiles-extras/pom.xml
tiles/framework/trunk/tiles-extras/src/main/java/org/apache/tiles/extras/renderer/OptionsRenderer.java
Modified: tiles/framework/trunk/tiles-extras/pom.xml
URL:
http://svn.apache.org/viewvc/tiles/framework/trunk/tiles-extras/pom.xml?rev=1368433&r1=1368432&r2=1368433&view=diff
==============================================================================
--- tiles/framework/trunk/tiles-extras/pom.xml (original)
+++ tiles/framework/trunk/tiles-extras/pom.xml Thu Aug 2 12:32:27 2012
@@ -112,6 +112,12 @@
</dependency>
<dependency>
+ <artifactId>guava</artifactId>
+ <groupId>com.google.guava</groupId>
+ <version>12.0.1</version>
+ </dependency>
+
+ <dependency>
<groupId>javax.servlet</groupId>
<artifactId>servlet-api</artifactId>
<scope>provided</scope>
Modified:
tiles/framework/trunk/tiles-extras/src/main/java/org/apache/tiles/extras/renderer/OptionsRenderer.java
URL:
http://svn.apache.org/viewvc/tiles/framework/trunk/tiles-extras/src/main/java/org/apache/tiles/extras/renderer/OptionsRenderer.java?rev=1368433&r1=1368432&r2=1368433&view=diff
==============================================================================
---
tiles/framework/trunk/tiles-extras/src/main/java/org/apache/tiles/extras/renderer/OptionsRenderer.java
(original)
+++
tiles/framework/trunk/tiles-extras/src/main/java/org/apache/tiles/extras/renderer/OptionsRenderer.java
Thu Aug 2 12:32:27 2012
@@ -23,11 +23,13 @@ package org.apache.tiles.extras.renderer
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.List;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
import org.apache.tiles.Attribute;
import org.apache.tiles.ListAttribute;
import org.apache.tiles.access.TilesAccess;
@@ -63,13 +65,21 @@ import org.slf4j.LoggerFactory;
* <p/>
* <p/>
* Currently only supports one occurrance of such an "option" pattern in the
attribute's value.
- *
+ * <p/>
* Limitation: "looking" for templates is implemented using
applicationContext.getResource(..)
* therefore the option values in the options list need to be visible as
applicationResources.
- *
+ * <p/>
+ * The attribute found and rendered is cached so to improve performance on
subsequent lookups.
+ * The default cache time-to-live is {@value #DEFAULT_CACHE_LIFE}, specified
by {@link #DEFAULT_CACHE_LIFE}.
+ * It can be customised by setting the system property {@value
#CACHE_LIFE_PROPERTY}, see {@link #CACHE_LIFE_PROPERTY}.
+ * Setting it to zero will disable the cache.
*/
public final class OptionsRenderer implements Renderer {
+ public static final String CACHE_LIFE_PROPERTY =
OptionsRenderer.class.getName() + ".cache_ttl_ms";
+
+ public static final long DEFAULT_CACHE_LIFE = 1000 * 60 * 5;
+
private static final Pattern OPTIONS_PATTERN
= Pattern.compile(Pattern.quote("{options[") + "(.+)" +
Pattern.quote("]}"));
@@ -114,7 +124,7 @@ public final class OptionsRenderer imple
if (done) { break; }
}
if (!done) {
- throw new IOException("None of the options existed for " + path);
+ throw new IOException("None of the options existed for " +
path);
}
} else {
renderer.render(path, request);
@@ -123,12 +133,11 @@ public final class OptionsRenderer imple
private boolean renderAttempt(final String template, final Request
request) throws IOException {
boolean result = false;
- if (!Cache.isTemplateMissing(template)) {
+ if (Cache.attemptTemplate(template)) {
try {
if (null != applicationContext.getResource(template)) { // can
throw FileNotFoundException !
renderer.render(template, request); // can throw
FileNotFoundException !
result = true;
- Cache.setIfAbsentTemplateFound(template, true);
}
} catch (FileNotFoundException ex) {
if (ex.getMessage().contains(template)) {
@@ -141,46 +150,40 @@ public final class OptionsRenderer imple
} catch (IOException ex) { //xxx ???
throw ex;
}
- Cache.setIfAbsentTemplateFound(template, false);
+ Cache.update(template, result);
}
return result;
}
private static final class Cache {
- /** It takes CACHE_LIFE milliseconds for any hot deployments to
register.
- */
- private static final ConcurrentMap<String,Boolean> TEMPLATE_EXISTS
- = new ConcurrentHashMap<String,Boolean>();
-
- private static volatile long cacheLastCleaned =
System.currentTimeMillis();
+ private static final long CACHE_LIFE =
Long.getLong(CACHE_LIFE_PROPERTY, DEFAULT_CACHE_LIFE);
- private static final String CACHE_LIFE_PROPERTY =
Cache.class.getName() + ".ttl_ms";
+ static {
+ LOG.info("cache_ttl_ms=" + CACHE_LIFE);
+ }
- /** Cache TTL be customised by a system property like
- * -Dorg.apache.tiles.extras.renderer.OptionsRenderer.Cache.ttl_ms=0
- *
- * The default is 5 minutes.
- * Setting it to zero disables all caching.
+ /** It takes CACHE_LIFE milliseconds for any hot deployments to
register.
*/
- private static final long CACHE_LIFE = null !=
Long.getLong(CACHE_LIFE_PROPERTY)
- ? Long.getLong(CACHE_LIFE_PROPERTY)
- : 1000 * 60 * 5;
-
- static boolean isTemplateMissing(final String template) {
- if (0 < CACHE_LIFE && System.currentTimeMillis() >
cacheLastCleaned + CACHE_LIFE) {
- cacheLastCleaned = System.currentTimeMillis();
- TEMPLATE_EXISTS.clear();
- return false;
- } else {
- return TEMPLATE_EXISTS.containsKey(template) &&
!TEMPLATE_EXISTS.get(template);
- }
+ private static final ConcurrentMap<String,Boolean> TEMPLATE_EXISTS =
CacheBuilder.newBuilder()
+ .expireAfterWrite(CACHE_LIFE, TimeUnit.MILLISECONDS)
+ .build(
+ new CacheLoader<String, Boolean>() {
+ @Override
+ public Boolean load(String key) {
+ throw new UnsupportedOperationException(
+ "illegal TEMPLATE_EXISTS.get(\"" + key
+ + "\") before TEMPLATE_EXISTS.containsKey(\""
+ key + "\")");
+ }
+ })
+ .asMap();
+
+ static boolean attemptTemplate(final String template) {
+ return !TEMPLATE_EXISTS.containsKey(template) ||
TEMPLATE_EXISTS.get(template);
}
- static void setIfAbsentTemplateFound(final String template, final
boolean found) {
- if (0 < CACHE_LIFE && !TEMPLATE_EXISTS.containsKey(template)) {
- TEMPLATE_EXISTS.putIfAbsent(template, found);
- }
+ static void update(final String template, final boolean found) {
+ TEMPLATE_EXISTS.putIfAbsent(template, found);
}
private Cache() {}
