You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tiles.apache.org by mc...@apache.org on 2012/08/02 14:32:27 UTC
svn commit: r1368433 - in /tiles/framework/trunk/tiles-extras: pom.xml
src/main/java/org/apache/tiles/extras/renderer/OptionsRenderer.java
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() {}