Repository: incubator-freemarker Updated Branches: refs/heads/3 ee8e20967 -> 06f4628d6
All CacheStorage-s must be thread safe from now on (removed ConcurrentCacheStorage interface) Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/06f4628d Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/06f4628d Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/06f4628d Branch: refs/heads/3 Commit: 06f4628d6aa1071b41523a00a5d779f6dcccc59a Parents: ee8e209 Author: ddekany <[email protected]> Authored: Fri Feb 17 19:14:38 2017 +0100 Committer: ddekany <[email protected]> Committed: Fri Feb 17 19:14:38 2017 +0100 ---------------------------------------------------------------------- .../debug/impl/RmiDebuggedEnvironmentImpl.java | 23 ++--- .../freemarker/core/debug/impl/SoftCache.java | 89 ++++++++++++++++++++ .../core/templateresolver/CacheStorage.java | 6 +- .../ConcurrentCacheStorage.java | 35 -------- .../impl/DefaultTemplateResolver.java | 39 ++------- .../templateresolver/impl/NullCacheStorage.java | 8 +- .../templateresolver/impl/SoftCacheStorage.java | 47 +---------- .../impl/StrongCacheStorage.java | 14 +-- src/manual/en_US/FM3-CHANGE-LOG.txt | 3 +- 9 files changed, 114 insertions(+), 150 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/06f4628d/src/main/java/org/apache/freemarker/core/debug/impl/RmiDebuggedEnvironmentImpl.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/debug/impl/RmiDebuggedEnvironmentImpl.java b/src/main/java/org/apache/freemarker/core/debug/impl/RmiDebuggedEnvironmentImpl.java index 3d7f1ab..4b798e5 100644 --- a/src/main/java/org/apache/freemarker/core/debug/impl/RmiDebuggedEnvironmentImpl.java +++ b/src/main/java/org/apache/freemarker/core/debug/impl/RmiDebuggedEnvironmentImpl.java @@ -44,23 +44,16 @@ import org.apache.freemarker.core.model.TemplateModel; import org.apache.freemarker.core.model.TemplateModelException; import org.apache.freemarker.core.model.impl.SimpleCollection; import org.apache.freemarker.core.model.impl.SimpleScalar; -import org.apache.freemarker.core.templateresolver.CacheStorage; -import org.apache.freemarker.core.templateresolver.impl.SoftCacheStorage; import org.apache.freemarker.core.util.UndeclaredThrowableException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -/** - */ -class RmiDebuggedEnvironmentImpl -extends - RmiDebugModelImpl -implements - DebuggedEnvironment { - private static final long serialVersionUID = 1L; +@SuppressWarnings("serial") +class RmiDebuggedEnvironmentImpl extends RmiDebugModelImpl implements DebuggedEnvironment { - private static final CacheStorage storage = new SoftCacheStorage(new IdentityHashMap()); - private static final Object idLock = new Object(); + private static final SoftCache CACHE = new SoftCache(new IdentityHashMap()); + private static final Object ID_LOCK = new Object(); + private static long nextId = 1; private static Set remotes = new HashSet(); @@ -70,14 +63,14 @@ implements private RmiDebuggedEnvironmentImpl(Environment env) throws RemoteException { super(new DebugEnvironmentModel(env), DebugModel.TYPE_ENVIRONMENT); - synchronized (idLock) { + synchronized (ID_LOCK) { id = nextId++; } } static synchronized Object getCachedWrapperFor(Object key) throws RemoteException { - Object value = storage.get(key); + Object value = CACHE.get(key); if (value == null) { if (key instanceof TemplateModel) { int extraTypes; @@ -98,7 +91,7 @@ implements } } if (value != null) { - storage.put(key, value); + CACHE.put(key, value); } if (value instanceof Remote) { remotes.add(value); http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/06f4628d/src/main/java/org/apache/freemarker/core/debug/impl/SoftCache.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/debug/impl/SoftCache.java b/src/main/java/org/apache/freemarker/core/debug/impl/SoftCache.java new file mode 100644 index 0000000..81b7215 --- /dev/null +++ b/src/main/java/org/apache/freemarker/core/debug/impl/SoftCache.java @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.freemarker.core.debug.impl; + +import java.lang.ref.Reference; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.SoftReference; +import java.util.Map; + +class SoftCache { + + private final ReferenceQueue queue = new ReferenceQueue(); + private final Map map; + + public SoftCache(Map backingMap) { + map = backingMap; + } + + public Object get(Object key) { + processQueue(); + Reference ref = (Reference) map.get(key); + return ref == null ? null : ref.get(); + } + + public void put(Object key, Object value) { + processQueue(); + map.put(key, new SoftValueReference(key, value, queue)); + } + + public void remove(Object key) { + processQueue(); + map.remove(key); + } + + public void clear() { + map.clear(); + processQueue(); + } + + /** + * Returns a close approximation of the number of cache entries. + */ + public int getSize() { + processQueue(); + return map.size(); + } + + private void processQueue() { + for (; ; ) { + SoftValueReference ref = (SoftValueReference) queue.poll(); + if (ref == null) { + return; + } + Object key = ref.getKey(); + map.remove(key); + } + } + + private static final class SoftValueReference extends SoftReference { + private final Object key; + + SoftValueReference(Object key, Object value, ReferenceQueue queue) { + super(value, queue); + this.key = key; + } + + Object getKey() { + return key; + } + } + +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/06f4628d/src/main/java/org/apache/freemarker/core/templateresolver/CacheStorage.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/templateresolver/CacheStorage.java b/src/main/java/org/apache/freemarker/core/templateresolver/CacheStorage.java index 3f86c1b..e9e1d4c 100644 --- a/src/main/java/org/apache/freemarker/core/templateresolver/CacheStorage.java +++ b/src/main/java/org/apache/freemarker/core/templateresolver/CacheStorage.java @@ -19,15 +19,11 @@ package org.apache.freemarker.core.templateresolver; -import org.apache.freemarker.core.templateresolver.impl.DefaultTemplateResolver; - /** * Cache storage abstracts away the storage aspects of a cache - associating * an object with a key, retrieval and removal via the key. It is actually a * small subset of the {@link java.util.Map} interface. - * The implementations can be coded in a non-threadsafe manner as the natural - * user of the cache storage, {@link DefaultTemplateResolver} does the necessary - * synchronization. + * The implementations must be thread safe. * * @see org.apache.freemarker.core.Configuration#setCacheStorage(CacheStorage) */ http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/06f4628d/src/main/java/org/apache/freemarker/core/templateresolver/ConcurrentCacheStorage.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/templateresolver/ConcurrentCacheStorage.java b/src/main/java/org/apache/freemarker/core/templateresolver/ConcurrentCacheStorage.java deleted file mode 100644 index 461ac2e..0000000 --- a/src/main/java/org/apache/freemarker/core/templateresolver/ConcurrentCacheStorage.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.freemarker.core.templateresolver; - -/** - * An optional interface for cache storage that knows whether it can be - * concurrently accessible without synchronization. - */ -public interface ConcurrentCacheStorage extends CacheStorage { - - /** - * Returns true if this instance of cache storage is concurrently - * accessible from multiple threads without synchronization. - * @return true if this instance of cache storage is concurrently - * accessible from multiple threads without synchronization. - */ - public boolean isConcurrent(); -} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/06f4628d/src/main/java/org/apache/freemarker/core/templateresolver/impl/DefaultTemplateResolver.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/templateresolver/impl/DefaultTemplateResolver.java b/src/main/java/org/apache/freemarker/core/templateresolver/impl/DefaultTemplateResolver.java index 225137f..3763d35 100644 --- a/src/main/java/org/apache/freemarker/core/templateresolver/impl/DefaultTemplateResolver.java +++ b/src/main/java/org/apache/freemarker/core/templateresolver/impl/DefaultTemplateResolver.java @@ -41,7 +41,6 @@ import org.apache.freemarker.core.ast.MarkReleaserTemplateSpecifiedEncodingHandl import org.apache.freemarker.core.ast.TemplateConfiguration; import org.apache.freemarker.core.ast.TemplateSpecifiedEncodingHandler; import org.apache.freemarker.core.templateresolver.CacheStorage; -import org.apache.freemarker.core.templateresolver.ConcurrentCacheStorage; import org.apache.freemarker.core.templateresolver.GetTemplateResult; import org.apache.freemarker.core.templateresolver.MalformedTemplateNameException; import org.apache.freemarker.core.templateresolver.TemplateConfigurationFactory; @@ -54,9 +53,9 @@ import org.apache.freemarker.core.templateresolver.TemplateLoadingSource; import org.apache.freemarker.core.templateresolver.TemplateLookupStrategy; import org.apache.freemarker.core.templateresolver.TemplateNameFormat; import org.apache.freemarker.core.templateresolver.TemplateResolver; +import org.apache.freemarker.core.util.UndeclaredThrowableException; import org.apache.freemarker.core.util._NullArgumentException; import org.apache.freemarker.core.util._StringUtil; -import org.apache.freemarker.core.util.UndeclaredThrowableException; import org.slf4j.Logger; /** @@ -94,7 +93,6 @@ public class DefaultTemplateResolver extends TemplateResolver { private final TemplateNameFormat templateNameFormat; private final TemplateConfigurationFactory templateConfigurations; - private final boolean isCacheStorageConcurrent; /** {@link Configuration#setTemplateUpdateDelayMilliseconds(long)} */ private long templateUpdateDelayMilliseconds = DEFAULT_TEMPLATE_UPDATE_DELAY_MILLIS; /** {@link Configuration#setLocalizedLookup(boolean)} */ @@ -168,8 +166,6 @@ public class DefaultTemplateResolver extends TemplateResolver { _NullArgumentException.check("cacheStorage", cacheStorage); this.cacheStorage = cacheStorage; - isCacheStorageConcurrent = cacheStorage instanceof ConcurrentCacheStorage && - ((ConcurrentCacheStorage) cacheStorage).isConcurrent(); _NullArgumentException.check("templateLookupStrategy", templateLookupStrategy); this.templateLookupStrategy = templateLookupStrategy; @@ -298,14 +294,7 @@ public class DefaultTemplateResolver extends TemplateResolver { : null; final CachedResultKey cacheKey = new CachedResultKey(name, locale, customLookupCondition, encoding, parseAsFTL); - CachedResult oldCachedResult; - if (isCacheStorageConcurrent) { - oldCachedResult = (CachedResult) cacheStorage.get(cacheKey); - } else { - synchronized (cacheStorage) { - oldCachedResult = (CachedResult) cacheStorage.get(cacheKey); - } - } + CachedResult oldCachedResult = (CachedResult) cacheStorage.get(cacheKey); final long now = System.currentTimeMillis(); @@ -365,7 +354,7 @@ public class DefaultTemplateResolver extends TemplateResolver { + "(source: " + newTemplateLoaderResult.getSource() + ")" + " as it hasn't been changed on the backing store."); } - putIntoCache(cacheKey, newCachedResult); + cacheStorage.put(cacheKey, newCachedResult); return (Template) newCachedResult.templateOrException; } else { if (newTemplateLoaderResult.getStatus() != TemplateLoadingResultStatus.OPENED) { @@ -442,7 +431,7 @@ public class DefaultTemplateResolver extends TemplateResolver { } newCachedResult.templateOrException = template; newCachedResult.version = templateLoaderResult.getVersion(); - putIntoCache(cacheKey, newCachedResult); + cacheStorage.put(cacheKey, newCachedResult); return template; } catch (RuntimeException e) { if (newCachedResult != null) { @@ -563,17 +552,7 @@ public class DefaultTemplateResolver extends TemplateResolver { cachedResult.templateOrException = e; cachedResult.source = null; cachedResult.version = null; - putIntoCache(cacheKey, cachedResult); - } - - private void putIntoCache(CachedResultKey tk, CachedResult cachedTemplate) { - if (isCacheStorageConcurrent) { - cacheStorage.put(tk, cachedTemplate); - } else { - synchronized (cacheStorage) { - cacheStorage.put(tk, cachedTemplate); - } - } + cacheStorage.put(cacheKey, cachedResult); } @SuppressWarnings("deprecation") @@ -811,13 +790,7 @@ public class DefaultTemplateResolver extends TemplateResolver { : null; CachedResultKey tk = new CachedResultKey(name, locale, customLookupCondition, encoding, parse); - if (isCacheStorageConcurrent) { - cacheStorage.remove(tk); - } else { - synchronized (cacheStorage) { - cacheStorage.remove(tk); - } - } + cacheStorage.remove(tk); if (debug) { LOG.debug(debugPrefix + "Template was removed from the cache, if it was there"); } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/06f4628d/src/main/java/org/apache/freemarker/core/templateresolver/impl/NullCacheStorage.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/templateresolver/impl/NullCacheStorage.java b/src/main/java/org/apache/freemarker/core/templateresolver/impl/NullCacheStorage.java index 64f51b0..9b564cf 100644 --- a/src/main/java/org/apache/freemarker/core/templateresolver/impl/NullCacheStorage.java +++ b/src/main/java/org/apache/freemarker/core/templateresolver/impl/NullCacheStorage.java @@ -21,7 +21,6 @@ package org.apache.freemarker.core.templateresolver.impl; import org.apache.freemarker.core.templateresolver.CacheStorage; import org.apache.freemarker.core.templateresolver.CacheStorageWithGetSize; -import org.apache.freemarker.core.templateresolver.ConcurrentCacheStorage; /** * A cache storage that doesn't store anything. Use this if you @@ -31,17 +30,12 @@ import org.apache.freemarker.core.templateresolver.ConcurrentCacheStorage; * * @since 2.3.17 */ -public class NullCacheStorage implements ConcurrentCacheStorage, CacheStorageWithGetSize { +public class NullCacheStorage implements CacheStorage, CacheStorageWithGetSize { /** * @since 2.3.22 */ public static final NullCacheStorage INSTANCE = new NullCacheStorage(); - - @Override - public boolean isConcurrent() { - return true; - } @Override public Object get(Object key) { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/06f4628d/src/main/java/org/apache/freemarker/core/templateresolver/impl/SoftCacheStorage.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/templateresolver/impl/SoftCacheStorage.java b/src/main/java/org/apache/freemarker/core/templateresolver/impl/SoftCacheStorage.java index 1c0bd81..9b7e359 100644 --- a/src/main/java/org/apache/freemarker/core/templateresolver/impl/SoftCacheStorage.java +++ b/src/main/java/org/apache/freemarker/core/templateresolver/impl/SoftCacheStorage.java @@ -22,16 +22,11 @@ package org.apache.freemarker.core.templateresolver.impl; import java.lang.ref.Reference; import java.lang.ref.ReferenceQueue; import java.lang.ref.SoftReference; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import org.apache.freemarker.core.templateresolver.CacheStorage; import org.apache.freemarker.core.templateresolver.CacheStorageWithGetSize; -import org.apache.freemarker.core.templateresolver.ConcurrentCacheStorage; -import org.apache.freemarker.core.util.UndeclaredThrowableException; /** * Soft cache storage is a cache storage that uses {@link SoftReference} objects to hold the objects it was passed, @@ -41,31 +36,16 @@ import org.apache.freemarker.core.util.UndeclaredThrowableException; * * @see org.apache.freemarker.core.Configuration#setCacheStorage(CacheStorage) */ -public class SoftCacheStorage implements ConcurrentCacheStorage, CacheStorageWithGetSize { - private static final Method atomicRemove = getAtomicRemoveMethod(); +public class SoftCacheStorage implements CacheStorage, CacheStorageWithGetSize { private final ReferenceQueue queue = new ReferenceQueue(); - private final Map map; - private final boolean concurrent; + private final ConcurrentMap map; /** * Creates an instance that uses a {@link ConcurrentMap} internally. */ public SoftCacheStorage() { - this(new ConcurrentHashMap()); - } - - /** - * Returns true if the underlying Map is a {@code ConcurrentMap}. - */ - @Override - public boolean isConcurrent() { - return concurrent; - } - - public SoftCacheStorage(Map backingMap) { - map = backingMap; - concurrent = map instanceof ConcurrentMap; + map = new ConcurrentHashMap(); } @Override @@ -111,17 +91,7 @@ public class SoftCacheStorage implements ConcurrentCacheStorage, CacheStorageWit return; } Object key = ref.getKey(); - if (concurrent) { - try { - atomicRemove.invoke(map, new Object[] { key, ref }); - } catch (IllegalAccessException e) { - throw new UndeclaredThrowableException(e); - } catch (InvocationTargetException e) { - throw new UndeclaredThrowableException(e); - } - } else if (map.get(key) == ref) { - map.remove(key); - } + map.remove(key, ref); } } @@ -138,13 +108,4 @@ public class SoftCacheStorage implements ConcurrentCacheStorage, CacheStorageWit } } - private static Method getAtomicRemoveMethod() { - try { - return Class.forName("java.util.concurrent.ConcurrentMap").getMethod("remove", new Class[] { Object.class, Object.class }); - } catch (ClassNotFoundException e) { - return null; - } catch (NoSuchMethodException e) { - throw new UndeclaredThrowableException(e); - } - } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/06f4628d/src/main/java/org/apache/freemarker/core/templateresolver/impl/StrongCacheStorage.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/templateresolver/impl/StrongCacheStorage.java b/src/main/java/org/apache/freemarker/core/templateresolver/impl/StrongCacheStorage.java index 8447db8..e2b9d0c 100644 --- a/src/main/java/org/apache/freemarker/core/templateresolver/impl/StrongCacheStorage.java +++ b/src/main/java/org/apache/freemarker/core/templateresolver/impl/StrongCacheStorage.java @@ -21,10 +21,10 @@ package org.apache.freemarker.core.templateresolver.impl; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import org.apache.freemarker.core.templateresolver.CacheStorage; import org.apache.freemarker.core.templateresolver.CacheStorageWithGetSize; -import org.apache.freemarker.core.templateresolver.ConcurrentCacheStorage; /** * Strong cache storage is a cache storage that simply wraps a {@link Map}. It holds a strong reference to all objects @@ -33,18 +33,10 @@ import org.apache.freemarker.core.templateresolver.ConcurrentCacheStorage; * * @see org.apache.freemarker.core.Configuration#setCacheStorage(CacheStorage) */ -public class StrongCacheStorage implements ConcurrentCacheStorage, CacheStorageWithGetSize { +public class StrongCacheStorage implements CacheStorage, CacheStorageWithGetSize { - private final Map map = new ConcurrentHashMap(); + private final ConcurrentMap map = new ConcurrentHashMap(); - /** - * Always returns {@code true}. - */ - @Override - public boolean isConcurrent() { - return true; - } - @Override public Object get(Object key) { return map.get(key); http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/06f4628d/src/manual/en_US/FM3-CHANGE-LOG.txt ---------------------------------------------------------------------- diff --git a/src/manual/en_US/FM3-CHANGE-LOG.txt b/src/manual/en_US/FM3-CHANGE-LOG.txt index 04dd6ab..341e499 100644 --- a/src/manual/en_US/FM3-CHANGE-LOG.txt +++ b/src/manual/en_US/FM3-CHANGE-LOG.txt @@ -83,4 +83,5 @@ the FreeMarer 3 changelog here: o.a.f.core.templateresolver subpackage name makes sense. - Marked most static utility classes as internal, and renamed them to start with "_" (for example StringUtils was renamed to _StringUtil, thus people won't accidentally use it when they wanted to autocomplete to Apache Commons StringUtil). -- Deleted o.a.f..core.util.DOMNodeModel \ No newline at end of file +- Deleted o.a.f..core.util.DOMNodeModel +- All CacheStorage-s must be thread safe from now on (removed ConcurrentCacheStorage marker interface) \ No newline at end of file
