You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by he...@apache.org on 2023/11/18 04:22:29 UTC
(commons-jexl) branch master updated: JEXL-414: added cache interface, synchronized & concurrent implementations and factory handling;
This is an automated email from the ASF dual-hosted git repository.
henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
The following commit(s) were added to refs/heads/master by this push:
new e4b6b395 JEXL-414: added cache interface, synchronized & concurrent implementations and factory handling;
e4b6b395 is described below
commit e4b6b3956ed4f7e1404e4523c6629216b2384476
Author: Henri Biestro <hb...@cloudera.com>
AuthorDate: Fri Nov 17 20:22:17 2023 -0800
JEXL-414: added cache interface, synchronized & concurrent implementations and factory handling;
---
RELEASE-NOTES.txt | 1 +
pom.xml | 5 +
src/changes/changes.xml | 5 +-
.../java/org/apache/commons/jexl3/JexlBuilder.java | 24 ++++-
.../java/org/apache/commons/jexl3/JexlCache.java | 85 +++++++++++++++
.../commons/jexl3/internal/ConcurrentCache.java | 58 ++++++++++
.../org/apache/commons/jexl3/internal/Engine.java | 12 ++-
.../apache/commons/jexl3/internal/SoftCache.java | 118 +++++++++++----------
.../commons/jexl3/internal/TemplateEngine.java | 5 +-
.../java/org/apache/commons/jexl3/CacheTest.java | 15 ++-
10 files changed, 264 insertions(+), 64 deletions(-)
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index 1dfb7d87..1d173c6f 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -37,6 +37,7 @@ New Features in 3.3.1:
Bugs Fixed in 3.3.1:
===================
+* JEXL-415: Incorrect template eval result
* JEXL-412: Ambiguous syntax between namespace function call and map object definition.
* JEXL-410: JexlFeatures: ctor does not enable all features
* JEXL-409: Disable LEXICAL should disable LEXICAL_SHADE
diff --git a/pom.xml b/pom.xml
index c5a534dd..04f3cf3b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -122,6 +122,11 @@
<version>2.10.1</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.googlecode.concurrentlinkedhashmap</groupId>
+ <artifactId>concurrentlinkedhashmap-lru</artifactId>
+ <version>1.4.2</version>
+ </dependency>
</dependencies>
<build>
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 9d3c4c38..80fc9107 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -42,9 +42,12 @@
Allow 'trailing commas' or ellipsis while defining array, map and set literals
</action>
<!-- FIX -->
+ <action dev="henrib" type="fix" issue="JEXL-415" due-to="Xu Pengcheng" >
+ Incorrect template eval result.
+ </action>
<action dev="henrib" type="fix" issue="JEXL-412" due-to="Xu Pengcheng" >
Ambiguous syntax between namespace function call and map object definition.
- </action>action>
+ </action>
<action dev="henrib" type="fix" issue="JEXL-410" due-to="sebb">
JexlFeatures: ctor does not enable all features
</action>
diff --git a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
index 01656fef..b67640d7 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
@@ -21,6 +21,7 @@ import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.Collection;
import java.util.Map;
+import java.util.function.IntFunction;
import org.apache.commons.jexl3.internal.Engine;
import org.apache.commons.jexl3.introspection.JexlPermissions;
@@ -132,6 +133,9 @@ public class JexlBuilder {
/** The cache size. */
private int cache = -1;
+ /** The cache class factory. */
+ private IntFunction<JexlCache<?,?>> cacheFactory = JexlCache.createConcurrent();
+
/** The stack overflow limit. */
private int stackOverflow = Integer.MAX_VALUE;
@@ -615,11 +619,29 @@ public class JexlBuilder {
return this;
}
+ /**
+ * Sets the expression cache size the engine will use.
+ *
+ * @param factory the function to produce a cache.
+ * @return this builder
+ */
+ public JexlBuilder cacheFactory(final IntFunction<JexlCache<?, ?>> factory) {
+ this.cacheFactory = factory;
+ return this;
+ }
+
/**
* @return the cache size
*/
public int cache() {
- return cache;
+ return cache;
+ }
+
+ /**
+ * @return the cache factory
+ */
+ public IntFunction<JexlCache<?, ?>> cacheFactory() {
+ return this.cacheFactory;
}
/**
diff --git a/src/main/java/org/apache/commons/jexl3/JexlCache.java b/src/main/java/org/apache/commons/jexl3/JexlCache.java
new file mode 100644
index 00000000..9bd9a734
--- /dev/null
+++ b/src/main/java/org/apache/commons/jexl3/JexlCache.java
@@ -0,0 +1,85 @@
+/*
+ * 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.commons.jexl3;
+
+import org.apache.commons.jexl3.internal.ConcurrentCache;
+import org.apache.commons.jexl3.internal.SoftCache;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.function.IntFunction;
+
+/**
+ * Caching scripts or templates interface.
+ * @param <K> source
+ * @param <V> script or template
+ */
+public interface JexlCache<K, V> {
+ /**
+ * Returns the cache size.
+ *
+ * @return the cache size
+ */
+ int size();
+
+ /**
+ * Clears the cache.
+ */
+ void clear();
+
+ /**
+ * Gets a value from cache.
+ *
+ * @param key the cache entry key
+ * @return the cache entry value
+ */
+ V get(K key);
+
+ /**
+ * Puts a value in cache.
+ *
+ * @param key the cache entry key
+ * @param script the cache entry value
+ */
+ V put(K key, V script);
+
+ /**
+ * Produces the cache entry set.
+ * <p>
+ * For implementations testing only
+ * </p>
+ * @return the cache entry list
+ */
+ default Collection<Map.Entry<K, V>> entries() {
+ return Collections.emptyList();
+ }
+
+ /**
+ * @return a synchronized cache factory amenable to low concurrency usage
+ */
+ static IntFunction<JexlCache<?,?>> createSynchronized() {
+ return SoftCache::new;
+ }
+
+ /**
+ * @return a concurrent cache factory amenable to high concurrency usage
+ */
+ static IntFunction<JexlCache<?,?>> createConcurrent() {
+ return ConcurrentCache::new;
+ }
+}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/ConcurrentCache.java b/src/main/java/org/apache/commons/jexl3/internal/ConcurrentCache.java
new file mode 100644
index 00000000..03a39166
--- /dev/null
+++ b/src/main/java/org/apache/commons/jexl3/internal/ConcurrentCache.java
@@ -0,0 +1,58 @@
+/*
+ * 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.commons.jexl3.internal;
+
+import com.googlecode.concurrentlinkedhashmap.ConcurrentLinkedHashMap;
+
+import java.lang.ref.SoftReference;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * A cache whose underlying map is a ConcurrentLinkedHashMap.
+ *
+ * @param <K> the cache key entry type
+ * @param <V> the cache key value type
+ */
+public class ConcurrentCache<K, V> extends SoftCache<K, V> {
+ /**
+ * Creates a new instance of a concurrent cache.
+ *
+ * @param theSize the cache size
+ */
+ public ConcurrentCache(final int theSize) {
+ super(theSize);
+ }
+
+ @Override
+ public Collection<Map.Entry<K, V>> entries() {
+ final SoftReference<Map<K, V>> ref = reference;
+ final Map<K, V> map = ref != null ? ref.get() : null;
+ return map == null? Collections.emptyList() : map.entrySet();
+ }
+
+ @Override
+ public <K, V> Map<K, V> createMap(final int cacheSize) {
+ return new ConcurrentLinkedHashMap.Builder<K, V>()
+ .concurrencyLevel(Runtime.getRuntime().availableProcessors())
+ .maximumWeightedCapacity(cacheSize)
+ .build();
+ }
+}
+
+
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Engine.java b/src/main/java/org/apache/commons/jexl3/internal/Engine.java
index fa304700..bb355b42 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Engine.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Engine.java
@@ -31,8 +31,10 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
+import java.util.function.IntFunction;
import java.util.function.Predicate;
+import org.apache.commons.jexl3.JexlCache;
import org.apache.commons.jexl3.JexlArithmetic;
import org.apache.commons.jexl3.JexlBuilder;
import org.apache.commons.jexl3.JexlContext;
@@ -160,7 +162,7 @@ public class Engine extends JexlEngine {
/**
* The expression cache.
*/
- protected final SoftCache<Source, ASTJexlScript> cache;
+ protected final JexlCache<Source, ASTJexlScript> cache;
/**
* The default jxlt engine.
*/
@@ -173,6 +175,10 @@ public class Engine extends JexlEngine {
* A cached version of the options.
*/
protected final JexlOptions options;
+ /**
+ * The cache factory method.
+ */
+ protected final IntFunction<JexlCache<?, ?>> cacheFactory;
/**
* Creates an engine with default arguments.
@@ -229,7 +235,9 @@ public class Engine extends JexlEngine {
this.scriptFeatures = new JexlFeatures(features).script(true).namespaceTest(nsTest);
this.charset = conf.charset();
// caching:
- this.cache = conf.cache() <= 0 ? null : new SoftCache<>(conf.cache());
+ IntFunction<JexlCache<?, ?>> factory = conf.cacheFactory();
+ this.cacheFactory = factory == null ? SoftCache::new : factory;
+ this.cache = (JexlCache<Source, ASTJexlScript>) (conf.cache() > 0 ? factory.apply(conf.cache()) : null);
this.cacheThreshold = conf.cacheThreshold();
if (uberspect == null) {
throw new IllegalArgumentException("uberspect can not be null");
diff --git a/src/main/java/org/apache/commons/jexl3/internal/SoftCache.java b/src/main/java/org/apache/commons/jexl3/internal/SoftCache.java
index f298959a..60c80884 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/SoftCache.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/SoftCache.java
@@ -16,25 +16,32 @@
*/
package org.apache.commons.jexl3.internal;
+import org.apache.commons.jexl3.JexlCache;
+
import java.lang.ref.SoftReference;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
/**
* A soft referenced cache.
* <p>
- * The actual cache is held through a soft reference, allowing it to be GCed
- * under memory pressure.</p>
+ * The actual cache is held through a soft reference, allowing it to be GCed
+ * under memory pressure.
+ * </p>
+ * <p>
+ * Note that the underlying map is a synchronized LinkedHashMap.
+ * The reason is that a get() will reorder elements (the LRU queue) and thus
+ * needs to be guarded to be thread-safe.
+ * </p>
*
* @param <K> the cache key entry type
* @param <V> the cache key value type
*/
-public class SoftCache<K, V> {
+public class SoftCache<K, V> implements JexlCache<K, V> {
/**
* The default cache load factor.
*/
@@ -42,24 +49,19 @@ public class SoftCache<K, V> {
/**
* The cache size.
*/
- private final int size;
+ protected final int size;
/**
* The soft reference to the cache map.
*/
- private SoftReference<Map<K, V>> ref;
- /**
- * The cache r/w lock.
- */
- private final ReadWriteLock lock;
+ protected volatile SoftReference<Map<K, V>> reference;
/**
* Creates a new instance of a soft cache.
*
* @param theSize the cache size
*/
- SoftCache(final int theSize) {
+ public SoftCache(final int theSize) {
size = theSize;
- lock = new ReentrantReadWriteLock();
}
/**
@@ -67,6 +69,7 @@ public class SoftCache<K, V> {
*
* @return the cache size
*/
+ @Override
public int size() {
return size;
}
@@ -74,12 +77,15 @@ public class SoftCache<K, V> {
/**
* Clears the cache.
*/
+ @Override
public void clear() {
- lock.writeLock().lock();
- try {
- ref = null;
- } finally {
- lock.writeLock().unlock();
+ final SoftReference<Map<K, V>> ref = reference;
+ if (ref != null ) {
+ reference = null;
+ final Map<K, V> map = ref.get();
+ if (map != null) {
+ map.clear();
+ }
}
}
@@ -89,14 +95,11 @@ public class SoftCache<K, V> {
* @param key the cache entry key
* @return the cache entry value
*/
+ @Override
public V get(final K key) {
- lock.readLock().lock();
- try {
- final Map<K, V> map = ref != null ? ref.get() : null;
- return map != null ? map.get(key) : null;
- } finally {
- lock.readLock().unlock();
- }
+ final SoftReference<Map<K, V>> ref = reference;
+ final Map<K, V> map = ref != null ? ref.get() : null;
+ return map != null ? map.get(key) : null;
}
/**
@@ -105,18 +108,21 @@ public class SoftCache<K, V> {
* @param key the cache entry key
* @param script the cache entry value
*/
- public void put(final K key, final V script) {
- lock.writeLock().lock();
- try {
- Map<K, V> map = ref != null ? ref.get() : null;
- if (map == null) {
- map = createCache(size);
- ref = new SoftReference<>(map);
+ @Override
+ public V put(final K key, final V script) {
+ SoftReference<Map<K, V>> ref = reference;
+ Map<K, V> map = ref != null ? ref.get() : null;
+ if (map == null) {
+ synchronized (this) {
+ ref = reference;
+ map = ref != null ? ref.get() : null;
+ if (map == null) {
+ map = createMap(size);
+ reference = new SoftReference<>(map);
+ }
}
- map.put(key, script);
- } finally {
- lock.writeLock().unlock();
}
+ return map.put(key, script);
}
/**
@@ -126,21 +132,20 @@ public class SoftCache<K, V> {
*
* @return the cache entry list
*/
- public List<Map.Entry<K, V>> entries() {
- lock.readLock().lock();
- try {
- final Map<K, V> map = ref != null ? ref.get() : null;
- if (map == null) {
- return Collections.emptyList();
- }
+ @Override
+ public Collection<Map.Entry<K, V>> entries() {
+ final SoftReference<Map<K, V>> ref = reference;
+ final Map<K, V> map = ref != null ? ref.get() : null;
+ if (map == null) {
+ return Collections.emptyList();
+ }
+ synchronized(map) {
final Set<Map.Entry<K, V>> set = map.entrySet();
final List<Map.Entry<K, V>> entries = new ArrayList<>(set.size());
for (final Map.Entry<K, V> e : set) {
entries.add(new SoftCacheEntry<>(e));
}
return entries;
- } finally {
- lock.readLock().unlock();
}
}
@@ -152,18 +157,20 @@ public class SoftCache<K, V> {
* @param cacheSize the cache size, must be > 0
* @return a Map usable as a cache bounded to the given size
*/
- public <K, V> Map<K, V> createCache(final int cacheSize) {
- return new java.util.LinkedHashMap<K, V>(cacheSize, LOAD_FACTOR, true) {
- /**
- * Serial version UID.
- */
- private static final long serialVersionUID = 1L;
-
- @Override
- protected boolean removeEldestEntry(final Map.Entry<K, V> eldest) {
- return super.size() > cacheSize;
+ public <K, V> Map<K, V> createMap(final int cacheSize) {
+ return Collections.synchronizedMap(
+ new java.util.LinkedHashMap<K, V>(cacheSize, LOAD_FACTOR, true) {
+ /**
+ * Serial version UID.
+ */
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected boolean removeEldestEntry(final Map.Entry<K, V> eldest) {
+ return super.size() > cacheSize;
+ }
}
- };
+ );
}
}
@@ -209,3 +216,4 @@ final class SoftCacheEntry<K, V> implements Map.Entry<K, V> {
}
}
+
diff --git a/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java b/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
index 6737f11e..1504123b 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
@@ -26,6 +26,7 @@ import java.util.Iterator;
import java.util.List;
import java.util.Set;
+import org.apache.commons.jexl3.JexlCache;
import org.apache.commons.jexl3.JexlContext;
import org.apache.commons.jexl3.JexlException;
import org.apache.commons.jexl3.JexlInfo;
@@ -42,7 +43,7 @@ import org.apache.commons.logging.Log;
*/
public final class TemplateEngine extends JxltEngine {
/** The TemplateExpression cache. */
- final SoftCache<String, TemplateExpression> cache;
+ final JexlCache<String, TemplateExpression> cache;
/** The JEXL engine instance. */
final Engine jexl;
/** The logger. */
@@ -69,7 +70,7 @@ public final class TemplateEngine extends JxltEngine {
final char deferred) {
this.jexl = aJexl;
this.logger = aJexl.logger;
- this.cache = new SoftCache<>(cacheSize);
+ this.cache = (JexlCache<String, TemplateExpression>) aJexl.cacheFactory.apply(cacheSize);
immediateChar = immediate;
deferredChar = deferred;
noscript = noScript;
diff --git a/src/test/java/org/apache/commons/jexl3/CacheTest.java b/src/test/java/org/apache/commons/jexl3/CacheTest.java
index cb1ccb2a..4ae8ddd4 100644
--- a/src/test/java/org/apache/commons/jexl3/CacheTest.java
+++ b/src/test/java/org/apache/commons/jexl3/CacheTest.java
@@ -525,9 +525,18 @@ public class CacheTest extends JexlTestCase {
3, 3, 3, 4, 4, 4, 0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 2, 2, 3, 3, 0
};
- private static final JexlEngine jexlCache = new JexlBuilder().cache(1024).debug(true).strict(true).create();
-
- private static final JexlEngine jexlNoCache = new JexlBuilder().cache(0).debug(true).strict(true).create();
+ private static final JexlEngine jexlCache = new JexlBuilder()
+ .cache(1024)
+ .cacheFactory(JexlCache.createSynchronized())
+ .debug(true)
+ .strict(true)
+ .create();
+
+ private static final JexlEngine jexlNoCache = new JexlBuilder()
+ .cache(0)
+ .debug(true)
+ .strict(true)
+ .create();
private static JexlEngine jexl = jexlCache;