feat: Use Caffeine for caching compiled scripts in JSR223 samplers instead of commons-collections4 LRUMap

src/core/src/main/java/org/apache/jmeter/util/JSR223TestElement.java

@@ -22,9 +22,8 @@
 import java.io.IOException;
 import java.io.Serializable;
 import java.nio.file.Files;
-import java.util.Collections;
-import java.util.Map;
 import java.util.Properties;
+import java.util.function.Function;
 
 import javax.script.Bindings;
 import javax.script.Compilable;
@@ -34,7 +33,6 @@
 import javax.script.ScriptException;
 
 import org.apache.commons.codec.digest.DigestUtils;
-import org.apache.commons.collections4.map.LRUMap;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.jmeter.samplers.SampleResult;
 import org.apache.jmeter.samplers.Sampler;
@@ -46,6 +44,9 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+
 /**
  * Base class for JSR223 Test elements
  */
@@ -58,15 +59,31 @@ public abstract class JSR223TestElement extends ScriptingTestElement
     /**
      * Cache of compiled scripts
      */
-    private static final Map<String, CompiledScript> compiledScriptsCache =
-            Collections.synchronizedMap(
-                    new LRUMap<>(JMeterUtils.getPropDefault("jsr223.compiled_scripts_cache_size", 100)));
+    private static final Cache<ScriptCacheKey, CompiledScript> COMPILED_SCRIPT_CACHE =
+            Caffeine
+                    .newBuilder()
+                    .maximumSize(JMeterUtils.getPropDefault("jsr223.compiled_scripts_cache_size", 100))
+                    .build();
+
+    /**
+     * Lambdas can't throw checked exceptions, so we wrap cache loading failure with a runtime one.
+     */
+    static class ScriptCompilationInvocationTargetException extends RuntimeException {
+        public ScriptCompilationInvocationTargetException(Throwable cause) {
+            super(cause);
+        }
+
+        @Override
+        public synchronized Throwable fillInStackTrace() {
+            return this;
+        }
+    }
 
     /** If not empty then script in ScriptText will be compiled and cached */
     private String cacheKey = "";
 
     /** md5 of the script, used as an unique key for the cache */
-    private String scriptMd5 = null;
+    private ScriptCacheKey scriptMd5;
 
     /**
      * Initialization On Demand Holder pattern
@@ -172,48 +189,41 @@ protected Object processFileOrScript(ScriptEngine scriptEngine, final Bindings p
                 && !"bsh.engine.BshScriptEngine".equals(scriptEngine.getClass().getName()); // NOSONAR // $NON-NLS-1$
         try {
             if (!StringUtils.isEmpty(getFilename())) {
-                if (scriptFile.exists() && scriptFile.canRead()) {
-                    if (supportsCompilable) {
-                        String newCacheKey = getScriptLanguage() + "#" + // $NON-NLS-1$
-                                scriptFile.getAbsolutePath() + "#" + // $NON-NLS-1$
-                                scriptFile.lastModified();
-                        CompiledScript compiledScript = compiledScriptsCache.get(newCacheKey);
-                        if (compiledScript == null) {
-                            synchronized (compiledScriptsCache) {
-                                compiledScript = compiledScriptsCache.get(newCacheKey);
-                                if (compiledScript == null) {
-                                    try (BufferedReader fileReader = Files.newBufferedReader(scriptFile.toPath())) {
-                                        compiledScript = ((Compilable) scriptEngine).compile(fileReader);
-                                        compiledScriptsCache.put(newCacheKey, compiledScript);
-                                    }
-                                }
-                            }
-                        }
-                        return compiledScript.eval(bindings);
-                    } else {
-                        try (BufferedReader fileReader = Files.newBufferedReader(scriptFile.toPath())) {
-                            return scriptEngine.eval(fileReader, bindings);
-                        }
-                    }
-                } else {
+                if (!scriptFile.isFile()) {
+                    throw new ScriptException("Script file '" + scriptFile.getAbsolutePath()
+                            + "' is not a file for element: " + getName());
+                }
+                if (!scriptFile.canRead()) {
                     throw new ScriptException("Script file '" + scriptFile.getAbsolutePath()
-                            + "' does not exist or is unreadable for element:" + getName());
+                            + "' is not readable for element:" + getName());
+                }
+                if (!supportsCompilable) {
+                    try (BufferedReader fileReader = Files.newBufferedReader(scriptFile.toPath())) {
+                        return scriptEngine.eval(fileReader, bindings);
+                    }
                 }
+                CompiledScript compiledScript;
+                ScriptCacheKey newCacheKey =
+                        ScriptCacheKey.ofFile(getScriptLanguage(), scriptFile.getAbsolutePath(), scriptFile.lastModified());
+                compiledScript = getCompiledScript(newCacheKey, key -> {
+                    try (BufferedReader fileReader = Files.newBufferedReader(scriptFile.toPath())) {
+                        return ((Compilable) scriptEngine).compile(fileReader);
+                    } catch (IOException | ScriptException e) {
+                        throw new ScriptCompilationInvocationTargetException(e);
+                    }
+                });
+                return compiledScript.eval(bindings);
             } else if (!StringUtils.isEmpty(getScript())) {
                 if (supportsCompilable &&
                         !ScriptingBeanInfoSupport.FALSE_AS_STRING.equals(cacheKey)) {
                     computeScriptMD5();
-                    CompiledScript compiledScript = compiledScriptsCache.get(this.scriptMd5);
-                    if (compiledScript == null) {
-                        synchronized (compiledScriptsCache) {
-                            compiledScript = compiledScriptsCache.get(this.scriptMd5);
-                            if (compiledScript == null) {
-                                compiledScript = ((Compilable) scriptEngine).compile(getScript());
-                                compiledScriptsCache.put(this.scriptMd5, compiledScript);
-                            }
+                    CompiledScript compiledScript = getCompiledScript(scriptMd5, key -> {
+                        try {
+                            return ((Compilable) scriptEngine).compile(getScript());
+                        } catch (ScriptException e) {
+                            throw new ScriptCompilationInvocationTargetException(e);
                         }
-                    }
-
+                    });
                     return compiledScript.eval(bindings);
                 } else {
                     return scriptEngine.eval(getScript(), bindings);
@@ -231,6 +241,30 @@ protected Object processFileOrScript(ScriptEngine scriptEngine, final Bindings p
         }
     }
 
+    private static <T extends ScriptCacheKey> CompiledScript getCompiledScript(
+            T newCacheKey,
+            Function<? super ScriptCacheKey, ? extends CompiledScript> compiler
+    ) throws IOException, ScriptException {
+        try {
+            CompiledScript compiledScript = COMPILED_SCRIPT_CACHE.get(newCacheKey, compiler);
+            if (compiledScript == null) {
+                throw new ScriptException("Script compilation returned null: " + newCacheKey);
+            }
+            return compiledScript;
+        } catch (ScriptCompilationInvocationTargetException e) {
+            Throwable cause = e.getCause();
+            if (cause instanceof IOException) {
+                cause.addSuppressed(new IllegalStateException("Unable to compile script " + newCacheKey));
+                throw (IOException) cause;
+            }
+            if (cause instanceof ScriptException) {
+                cause.addSuppressed(new IllegalStateException("Unable to compile script " + newCacheKey));
+                throw (ScriptException) cause;
+            }
+            throw e;
+        }
+    }
+
     /**
      * @return boolean true if element is not compilable or if compilation succeeds
      * @throws IOException if script is missing
@@ -273,7 +307,7 @@ public boolean compile()
     private void computeScriptMD5() {
         // compute the md5 of the script if needed
         if(scriptMd5 == null) {
-            scriptMd5 = DigestUtils.md5Hex(getScript());
+            scriptMd5 = ScriptCacheKey.ofString(DigestUtils.md5Hex(getScript()));
         }
     }
 
@@ -320,7 +354,7 @@ public void testEnded() {
      */
     @Override
     public void testEnded(String host) {
-        compiledScriptsCache.clear();
+        COMPILED_SCRIPT_CACHE.invalidateAll();
         this.scriptMd5 = null;
     }
 

src/core/src/main/java/org/apache/jmeter/util/ScriptCacheKey.java

@@ -0,0 +1,116 @@
+/*
+ * 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.jmeter.util;
+
+import java.util.Objects;
+
+interface ScriptCacheKey {
+    /**
+     * Creates a script key from a string. Note: do not use concatenation to build a key. Prefer creating a subclass.
+     * @param key cache key
+     * @return cache key
+     */
+    static ScriptCacheKey ofString(String key) {
+        return new StringScriptCacheKey(key);
+    }
+
+    /**
+     * Creates a cache key for a file contents assuming its last modified date is up to date.
+     * @param language script language
+     * @param absolutePath absolute path of the file
+     * @param lastModified file last modification date
+     * @return cache key
+     */
+    static ScriptCacheKey ofFile(String language, String absolutePath, long lastModified) {
+        return new FileScriptCacheKey(language, absolutePath, lastModified);
+    }
+
+    final class StringScriptCacheKey implements ScriptCacheKey {
+        final String contents;
+
+        StringScriptCacheKey(String contents) {
+            this.contents = contents;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) {
+                return true;
+            }
+            if (o == null || getClass() != o.getClass()) {
+                return false;
+            }
+            StringScriptCacheKey that = (StringScriptCacheKey) o;
+            return Objects.equals(contents, that.contents);
+        }
+
+        @Override
+        public int hashCode() {
+            return contents.hashCode();
+        }
+
+        @Override
+        public String toString() {
+            return "StringScriptCacheKey{" +
+                    "contents='" + contents + '\'' +
+                    '}';
+        }
+    }
+
+    final class FileScriptCacheKey implements ScriptCacheKey {
+        final String language;
+        final String absolutePath;
+        final long lastModified;
+
+        FileScriptCacheKey(String language, String absolutePath, long lastModified) {
+            this.language = language;
+            this.absolutePath = absolutePath;
+            this.lastModified = lastModified;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) {
+                return true;
+            }
+            if (o == null || getClass() != o.getClass()) {
+                return false;
+            }
+            FileScriptCacheKey that = (FileScriptCacheKey) o;
+            return Objects.equals(language, that.language) && Objects.equals(absolutePath, that.absolutePath) && lastModified == that.lastModified;
+        }
+
+        @Override
+        public int hashCode() {
+            int hash = 1;
+            hash = 31 * hash + language.hashCode();
+            hash = 31 * hash + absolutePath.hashCode();
+            hash = 31 * hash + Long.hashCode(lastModified);
+            return hash;
+        }
+
+        @Override
+        public String toString() {
+            return "ScriptCacheKey{" +
+                    "language='" + language + '\'' +
+                    ", absolutePath='" + absolutePath + '\'' +
+                    ", lastModified=" + lastModified +
+                    '}';
+        }
+    }
+}

xdocs/changes.xml

@@ -79,6 +79,7 @@ Summary
 
 <h3>Other samplers</h3>
 <ul>
+  <li><pr>5909</pr> Use Caffeine for caching compiled scripts in JSR223 samplers instead of commons-collections4 LRUMap</li>
 </ul>
 
 <h3>Controllers</h3>