Author: thomas.heute(a)jboss.com Date: 2010-04-07 03:39:09 -0400 (Wed, 07 Apr 2010) New Revision: 2515 Removed: portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/Entry.java Modified: portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureExoCache.java portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/Loader.java portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/ConcurrentGetWhenPutTestCase.java portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/FutureMap.java portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/GetTestCase.java portal/branches/EPP_5_0_Branch/component/scripting/src/main/java/org/exoplatform/groovyscript/text/TemplateService.java Log: JBEPP-265: Handle gracefully concurrent compilation of the same template Deleted: portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/Entry.java =================================================================== --- portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/Entry.java 2010-04-07 06:54:06 UTC (rev 2514) +++ portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/Entry.java 2010-04-07 07:39:09 UTC (rev 2515) @@ -1,58 +0,0 @@ -/* - * Copyright (C) 2009 eXo Platform SAS. - * - * This is free software; you can redistribute it and/or modify it - * under the terms of the GNU Lesser General Public License as - * published by the Free Software Foundation; either version 2.1 of - * the License, or (at your option) any later version. - * - * This software is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this software; if not, write to the Free - * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA - * 02110-1301 USA, or see the FSF site: http://www.fsf.org. - */ - -package org.exoplatform.commons.cache.future; - -import java.io.Serializable; - -/** - * @author <a href="mailto:julien.viet@exoplatform.com">Julien Viet</a> - * @version $Revision$ - */ -public class Entry<V> implements Serializable -{ - - public static <V> Entry<V> create(V v) - { - return new Entry<V>(v); - } - - /** . */ - private final V value; - - private Entry(V value) - { - if (value == null) - { - throw new NullPointerException(); - } - this.value = value; - } - - public V getValue() - { - return value; - } - - @Override - public String toString() - { - return "Entry[" + value + "]"; - } -} Modified: portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java =================================================================== --- portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java 2010-04-07 06:54:06 UTC (rev 2514) +++ portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java 2010-04-07 07:39:09 UTC (rev 2515) @@ -30,6 +30,9 @@ * * @author <a href="mailto:julien.viet@exoplatform.com">Julien Viet</a> * @version $Revision$ + * @param <K> the key type parameter + * @param <V> the value type parameter + * @param <C> the context type parameter */ public abstract class FutureCache<K, V, C> { @@ -38,7 +41,7 @@ private final Loader<K, V, C> loader; /** . */ - private final ConcurrentMap<K, FutureTask<Entry<V>>> local; + private final ConcurrentMap<K, FutureTask<V>> futureEntries; /** . */ private final Logger log = LoggerFactory.getLogger(FutureCache.class); @@ -46,26 +49,35 @@ public FutureCache(Loader<K, V, C> loader) { this.loader = loader; - this.local = new ConcurrentHashMap<K, FutureTask<Entry<V>>>(); + this.futureEntries = new ConcurrentHashMap<K, FutureTask<V>>(); } - protected abstract Entry<V> get(K key); + protected abstract V get(K key); - protected abstract void put(K key, Entry<V> entry); + protected abstract void put(K key, V value); - public V get(final C context, final K key) + /** + * Perform a cache lookup for the specified key within the specified context. + * When the value cannot be loaded (because it does not exist or it failed or anything else that + * does not come to my mind), the value null is returned. + * + * @param context the context in which the resource is accessed + * @param key the key identifying the resource + * @return the value + */ + public final V get(final C context, final K key) { // First we try a simple cache get - Entry<V> entry = get(key); + V value = get(key); // If it does not succeed then we go through a process that will avoid to load // the same resource concurrently - if (entry == null) + if (value == null) { // Create our future - FutureTask<Entry<V>> future = new FutureTask<Entry<V>>(new Callable<Entry<V>>() + FutureTask<V> future = new FutureTask<V>(new Callable<V>() { - public Entry<V> call() throws Exception + public V call() throws Exception { // Retrieve the value from the loader V value = loader.retrieve(context, key); @@ -73,14 +85,11 @@ // if (value != null) { - // Create the entry - Entry<V> entry = Entry.create(value); - // Cache it, it is made available to other threads (unless someone removes it) - put(key, entry); + put(key, value); - // Return entry - return entry; + // Return value + return value; } else { @@ -89,15 +98,15 @@ } }); - // Was our means that we inserted in the local + // This boolean means we inserted in the local boolean inserted = true; // try { - FutureTask<Entry<V>> phantom = local.putIfAbsent(key, future); + FutureTask<V> phantom = futureEntries.putIfAbsent(key, future); - // Use the entry that could have been inserted by another thread + // Use the value that could have been inserted by another thread if (phantom != null) { future = phantom; @@ -108,8 +117,8 @@ future.run(); } - // Returns the entry - entry = future.get(); + // Returns the value + value = future.get(); } catch (ExecutionException e) { @@ -124,12 +133,12 @@ // Clean up the per key map but only if our insertion succeeded and with our future if (inserted) { - local.remove(key, future); + futureEntries.remove(key, future); } } } // - return entry != null ? entry.getValue() : null; + return value; } } Modified: portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureExoCache.java =================================================================== --- portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureExoCache.java 2010-04-07 06:54:06 UTC (rev 2514) +++ portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureExoCache.java 2010-04-07 07:39:09 UTC (rev 2515) @@ -31,9 +31,9 @@ { /** . */ - private final ExoCache<K, Entry<V>> cache; + private final ExoCache<K, V> cache; - public FutureExoCache(Loader<K, V, C> loader, ExoCache<K, Entry<V>> cache) + public FutureExoCache(Loader<K, V, C> loader, ExoCache<K, V> cache) { super(loader); @@ -42,13 +42,13 @@ } @Override - protected Entry<V> get(K key) + protected V get(K key) { return cache.get(key); } @Override - protected void put(K key, Entry<V> entry) + protected void put(K key, V entry) { cache.put(key, entry); } Modified: portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/Loader.java =================================================================== --- portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/Loader.java 2010-04-07 06:54:06 UTC (rev 2514) +++ portal/branches/EPP_5_0_Branch/component/common/src/main/java/org/exoplatform/commons/cache/future/Loader.java 2010-04-07 07:39:09 UTC (rev 2515) @@ -19,15 +19,27 @@ package org.exoplatform.commons.cache.future; -import java.io.Serializable; - /** + * The loader interface is used by the future cache to retrieves the value from the key when it does not exist. + * * @author <a href="mailto:julien.viet@exoplatform.com">Julien Viet</a> * @version $Revision$ + * @param <K> the key type parameter + * @param <V> the value type parameter + * @param <C> the context type parameter */ public interface Loader<K, V, C> { + /** + * Retrieves the value from the key within the specified context. If the resource is not found then the value + * null must be returned. + * + * @param context the context + * @param key the key + * @return the value + * @throws Exception any exception that would prevent the value to be loaded + */ V retrieve(C context, K key) throws Exception; } Modified: portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/ConcurrentGetWhenPutTestCase.java =================================================================== --- portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/ConcurrentGetWhenPutTestCase.java 2010-04-07 06:54:06 UTC (rev 2514) +++ portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/ConcurrentGetWhenPutTestCase.java 2010-04-07 07:39:09 UTC (rev 2515) @@ -21,8 +21,6 @@ import junit.framework.AssertionFailedError; import junit.framework.TestCase; -import org.exoplatform.commons.cache.future.Entry; -import org.exoplatform.commons.cache.future.FutureCache; import org.gatein.common.util.Tools; import java.util.*; @@ -39,13 +37,13 @@ private AssertionFailedError failure; /** . */ - private LinkedList<String> events = new LinkedList<String>(); + private List<String> events = Collections.synchronizedList(new LinkedList<String>()); FutureCache<String, String, Callable<String>> futureCache = new FutureCache<String, String, Callable<String>>(new StringLoader()) { @Override - protected Entry<String> get(String key) + protected String get(String key) { if (key == key1) { @@ -53,7 +51,7 @@ { failure = new AssertionFailedError(); } - events.addLast("get/key1"); + events.add("get/key1"); } else if (key == key2) { @@ -61,7 +59,7 @@ { failure = new AssertionFailedError(); } - events.addLast("get/key2"); + events.add("get/key2"); } else { @@ -71,13 +69,13 @@ } @Override - protected void put(String key, Entry<String> entry) + protected void put(String key, String value) { if (key == key1) { if (Thread.currentThread() == thread1) { - events.addLast("begin_put/key1/" + entry); + events.add("begin_put/key1/" + value); // thread2.start(); @@ -89,7 +87,7 @@ } // - events.addLast("end_put/key1"); + events.add("end_put/key1"); } else { @@ -118,11 +116,11 @@ { public String call() throws Exception { - events.addLast("call/key1"); + events.add("call/key1"); return "foo_value_1"; } }, key1); - events.addLast("retrieved/key1/" + v); + events.add("retrieved/key1/" + v); } }; @@ -139,7 +137,7 @@ return "foo_value_2"; } }, key2); - events.addLast("retrieved/key2/" + v); + events.add("retrieved/key2/" + v); } }; @@ -161,7 +159,7 @@ List<String> expectedEvents = Arrays.asList( "get/key1", "call/key1", - "begin_put/key1/Entry[foo_value_1]", + "begin_put/key1/foo_value_1", "get/key2", "end_put/key1" ); Modified: portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/FutureMap.java =================================================================== --- portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/FutureMap.java 2010-04-07 06:54:06 UTC (rev 2514) +++ portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/FutureMap.java 2010-04-07 07:39:09 UTC (rev 2515) @@ -19,10 +19,6 @@ package org.exoplatform.commons.cache.future; -import org.exoplatform.commons.cache.future.Entry; -import org.exoplatform.commons.cache.future.FutureCache; -import org.exoplatform.commons.cache.future.Loader; - import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -35,25 +31,25 @@ { /** . */ - final Map<String, Entry<String>> data; + final Map<String, String> data; public FutureMap(Loader<String, String, C> loader) { super(loader); // - this.data = Collections.synchronizedMap(new HashMap<String, Entry<String>>()); + this.data = Collections.synchronizedMap(new HashMap<String, String>()); } @Override - protected Entry<String> get(String key) + protected String get(String key) { return data.get(key); } @Override - protected void put(String key, Entry<String> entry) + protected void put(String key, String value) { - data.put(key, entry); + data.put(key, value); } } Modified: portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/GetTestCase.java =================================================================== --- portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/GetTestCase.java 2010-04-07 06:54:06 UTC (rev 2514) +++ portal/branches/EPP_5_0_Branch/component/common/src/test/java/org/exoplatform/commons/cache/future/GetTestCase.java 2010-04-07 07:39:09 UTC (rev 2515) @@ -40,7 +40,7 @@ return "foo_value"; } }, "foo")); - Assert.assertEquals("foo_value", futureCache.data.get("foo").getValue()); + Assert.assertEquals("foo_value", futureCache.data.get("foo")); } public void testNullValue() Modified: portal/branches/EPP_5_0_Branch/component/scripting/src/main/java/org/exoplatform/groovyscript/text/TemplateService.java =================================================================== --- portal/branches/EPP_5_0_Branch/component/scripting/src/main/java/org/exoplatform/groovyscript/text/TemplateService.java 2010-04-07 06:54:06 UTC (rev 2514) +++ portal/branches/EPP_5_0_Branch/component/scripting/src/main/java/org/exoplatform/groovyscript/text/TemplateService.java 2010-04-07 07:39:09 UTC (rev 2515) @@ -22,7 +22,6 @@ import groovy.lang.Writable; import groovy.text.Template; -import org.exoplatform.commons.cache.future.Entry; import org.exoplatform.commons.cache.future.FutureCache; import org.exoplatform.commons.cache.future.FutureExoCache; import org.exoplatform.commons.cache.future.Loader; @@ -59,7 +58,7 @@ private GroovyTemplateEngine engine_; - private ExoCache<ResourceKey, Entry<GroovyTemplate>> templatesCache_; + private ExoCache<ResourceKey, GroovyTemplate> templatesCache_; private TemplateStatisticService statisticService; @@ -169,7 +168,7 @@ getTemplatesCache().remove(resourceId); } - public ExoCache<ResourceKey, Entry<GroovyTemplate>> getTemplatesCache() + public ExoCache<ResourceKey, GroovyTemplate> getTemplatesCache() { return templatesCache_; } @@ -219,13 +218,9 @@ try { ArrayList<String> list = new ArrayList<String>(); - for (Entry<GroovyTemplate> entry : templatesCache_.getCachedObjects()) + for (GroovyTemplate template : templatesCache_.getCachedObjects()) { - GroovyTemplate template = entry.getValue(); - if (template != null) - { - list.add(template.getId()); - } + list.add(template.getId()); } return list.toArray(new String[list.size()]); }