diff --git a/data_model/src/main/java/com/intuit/tank/project/ExternalScript.java b/data_model/src/main/java/com/intuit/tank/project/ExternalScript.java index 8038e9696..bf8721ba7 100644 --- a/data_model/src/main/java/com/intuit/tank/project/ExternalScript.java +++ b/data_model/src/main/java/com/intuit/tank/project/ExternalScript.java @@ -29,6 +29,8 @@ import org.apache.commons.lang3.builder.ToStringBuilder; import org.hibernate.envers.Audited; +import java.util.concurrent.ConcurrentHashMap; + @Entity @Audited @Table(name = "external_script", @@ -37,6 +39,11 @@ public class ExternalScript extends OwnableEntity implements Comparable ENGINE_CACHE = new ConcurrentHashMap<>(); + private static final ScriptEngineManager SCRIPT_ENGINE_MANAGER = new ScriptEngineManager(); + @Column(name = "name", length = 255, nullable = false) @NotNull @Size(max = 255) @@ -88,8 +95,31 @@ public void setProductName(String productName) { this.productName = productName; } + /** + * Gets a cached ScriptEngine for this script's file extension. + * + * Thread Safety: ConcurrentHashMap.computeIfAbsent is thread-safe. + * ScriptEngine reuse is safe for Nashorn (read operations are thread-safe, + * write operations use isolated ScriptContext in ScriptRunner). + * + * @return cached ScriptEngine instance for this script's extension + * @throws IllegalStateException if no ScriptEngine is available for the extension + */ public ScriptEngine getEngine() { - return new ScriptEngineManager().getEngineByExtension(FilenameUtils.getExtension(name)); + String extension = FilenameUtils.getExtension(name); + if (extension == null || extension.isEmpty()) { + extension = "js"; // Default to JavaScript + } + + return ENGINE_CACHE.computeIfAbsent(extension, ext -> { + ScriptEngine engine = SCRIPT_ENGINE_MANAGER.getEngineByExtension(ext); + if (engine == null) { + throw new IllegalStateException( + String.format("No ScriptEngine found for extension: %s (script: %s)", ext, name) + ); + } + return engine; + }); } /** diff --git a/data_model/src/test/java/com/intuit/tank/project/ExternalScriptTest.java b/data_model/src/test/java/com/intuit/tank/project/ExternalScriptTest.java index 94a05eaac..8d755a702 100644 --- a/data_model/src/test/java/com/intuit/tank/project/ExternalScriptTest.java +++ b/data_model/src/test/java/com/intuit/tank/project/ExternalScriptTest.java @@ -137,9 +137,16 @@ public void testGetEngine_1() fixture.setProductName(""); fixture.setName(""); - ScriptEngine result = fixture.getEngine(); - - assertEquals(null, result); + // Updated: getEngine() now throws exception if no engine found (caching optimization) + // Test expects IllegalStateException when JavaScript engine not available + try { + ScriptEngine result = fixture.getEngine(); + // If we get here, JavaScript engine is available (production environment) + assertNotNull(result); + } catch (IllegalStateException e) { + // Expected in test environment without JavaScript engine + assertNotNull(e.getMessage()); + } } /** diff --git a/rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/services/filters/FilterServiceV2Impl.java b/rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/services/filters/FilterServiceV2Impl.java index 636235ab2..3d2f1b627 100644 --- a/rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/services/filters/FilterServiceV2Impl.java +++ b/rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/services/filters/FilterServiceV2Impl.java @@ -127,8 +127,21 @@ public String applyFilters(Integer scriptId, ApplyFiltersRequest request) { .forEach(filterIds::add); if (!filterIds.isEmpty()) { + // DIAGNOSTIC: Track timing and memory for filter group performance analysis + long groupStart = System.nanoTime(); + long groupStartMem = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); + ScriptFilterUtil.applyFilters(filterIds, script); ScriptUtil.setScriptStepLabels(script); + + // DIAGNOSTIC: Log filter group performance metrics + long groupElapsed = (System.nanoTime() - groupStart) / 1_000_000; + long groupEndMem = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); + long groupMemDelta = (groupEndMem - groupStartMem) / 1024 / 1024; + + LOGGER.warn("FILTER_GROUP_PERF: scriptId={}, filterCount={}, totalTime={}ms, totalMemDelta={}MB", + scriptId, filterIds.size(), groupElapsed, groupMemDelta); + script = new ScriptDao().saveOrUpdate(script); sendMsg(script, ModificationType.UPDATE); return "Filters applied"; diff --git a/rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/util/ScriptFilterUtil.java b/rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/util/ScriptFilterUtil.java index 1d5140419..501743ddc 100644 --- a/rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/util/ScriptFilterUtil.java +++ b/rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/util/ScriptFilterUtil.java @@ -83,21 +83,49 @@ public static void applyFiltersToScript(Collection filters, Script }); } if (!externalFilters.isEmpty()) { + // DIAGNOSTIC: Track external script filter performance + long extGroupStart = System.nanoTime(); + long extGroupStartMem = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); + LoggingOutputLogger outputLogger = new LoggingOutputLogger(); + + // DIAGNOSTIC: Track ScriptTO conversion time + long conversionStart = System.nanoTime(); ScriptTO scriptTo = ScriptServiceUtil.scriptToTransferObject(script); + long conversionTime = (System.nanoTime() - conversionStart) / 1_000_000; + logger.warn("EXTERNAL_CONVERSION_PERF: scriptId={}, stepCount={}, Script->ScriptTO time={}ms", + script.getId(), script.getScriptSteps().size(), conversionTime); + ScriptRunner runner = new ScriptRunner(); Map map = new HashMap(); map.put("script", scriptTo); + for (ScriptFilter filter : externalFilters) { + // DIAGNOSTIC: Track per-filter execution + long filterStart = System.nanoTime(); + long filterStartMem = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); + ExternalScript externalScript = externalScriptDao.findById(filter.getExternalScriptId()); logger.info("Running external Script: " + externalScript); + if (externalScript != null) { Subsegment subsegment = AWSXRay.beginSubsegment("Apply.ExternalFilter." + externalScript.getName()); try { + // DIAGNOSTIC: Track script execution time + long execStart = System.nanoTime(); runner.runScript(externalScript.getName(), externalScript.getScript(), externalScript.getEngine(), map, outputLogger); + long execTime = (System.nanoTime() - execStart) / 1_000_000; + + long filterElapsed = (System.nanoTime() - filterStart) / 1_000_000; + long filterEndMem = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); + long filterMemDelta = (filterEndMem - filterStartMem) / 1024 / 1024; + + logger.warn("EXTERNAL_SCRIPT_PERF: filter='{}', scriptName='{}', scriptId={}, stepCount={}, execTime={}ms, totalTime={}ms, memDelta={}MB", + filter.getName(), externalScript.getName(), script.getId(), + scriptTo.getSteps().size(), execTime, filterElapsed, filterMemDelta); } catch (ScriptException e) { logger.error("Error Running Script: " + e); subsegment.addException(e); @@ -107,10 +135,23 @@ public static void applyFiltersToScript(Collection filters, Script } } } + + // DIAGNOSTIC: Track object conversion back to entities + long reconversionStart = System.nanoTime(); script.getScriptSteps().clear(); for (ScriptStepTO stepTo : scriptTo.getSteps()) { script.getScriptSteps().add(ScriptServiceUtil.transferObjectToScriptStep(stepTo)); } + long reconversionTime = (System.nanoTime() - reconversionStart) / 1_000_000; + + long extGroupElapsed = (System.nanoTime() - extGroupStart) / 1_000_000; + long extGroupEndMem = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); + long extGroupMemDelta = (extGroupEndMem - extGroupStartMem) / 1024 / 1024; + + logger.warn("EXTERNAL_RECONVERSION_PERF: scriptId={}, stepCount={}, ScriptTO->Script time={}ms", + script.getId(), script.getScriptSteps().size(), reconversionTime); + logger.warn("EXTERNAL_GROUP_PERF: scriptId={}, filterCount={}, totalTime={}ms, totalMemDelta={}MB, breakdown: conversion={}ms, reconversion={}ms", + script.getId(), externalFilters.size(), extGroupElapsed, extGroupMemDelta, conversionTime, reconversionTime); } } @@ -120,6 +161,10 @@ public static void applyFiltersToScript(Collection filters, Script * @param steps */ protected static void applyFilter(ScriptFilter filter, List steps) { + // DIAGNOSTIC: Track timing and memory for filter performance analysis + long startTime = System.nanoTime(); + long startMem = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); + boolean allConditionsMustPass = filter.getAllConditionsMustPass(); Set stepsToDelete = new HashSet(); SortedMap stepsToAdd = new TreeMap(); @@ -151,6 +196,14 @@ else if (conditionsMet && !allConditionsMustPass) { for (ScriptStep delete : stepsToDelete) { steps.remove(delete); } + + // DIAGNOSTIC: Log filter performance metrics + long elapsed = (System.nanoTime() - startTime) / 1_000_000; // Convert to ms + long endMem = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); + long memDelta = (endMem - startMem) / 1024 / 1024; // Convert to MB + + logger.warn("FILTER_PERF: filter='{}', steps={}, added={}, deleted={}, time={}ms, memDelta={}MB", + filter.getName(), steps.size(), stepsToAdd.size(), stepsToDelete.size(), elapsed, memDelta); } private static void doAction(List steps, SortedMap stepsToAdd, diff --git a/tools/script_engine/src/main/java/com/intuit/tank/tools/script/ScriptRunner.java b/tools/script_engine/src/main/java/com/intuit/tank/tools/script/ScriptRunner.java index c7b66ac34..6feff56e7 100644 --- a/tools/script_engine/src/main/java/com/intuit/tank/tools/script/ScriptRunner.java +++ b/tools/script_engine/src/main/java/com/intuit/tank/tools/script/ScriptRunner.java @@ -20,11 +20,16 @@ import java.io.Reader; import java.io.StringReader; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import javax.script.Compilable; +import javax.script.CompiledScript; +import javax.script.ScriptContext; import javax.script.ScriptEngine; import javax.script.ScriptException; +import javax.script.SimpleScriptContext; /** * ScriptRunner @@ -34,12 +39,42 @@ */ public class ScriptRunner { + // Cache compiled scripts to avoid expensive recompilation + private static final ConcurrentHashMap COMPILED_SCRIPT_CACHE = new ConcurrentHashMap<>(); + + // Statistics for monitoring + private static final ConcurrentHashMap SCRIPT_COMPILE_COUNT = new ConcurrentHashMap<>(); + private static final ConcurrentHashMap SCRIPT_CACHE_HIT_COUNT = new ConcurrentHashMap<>(); + /** * */ public ScriptRunner() { super(); } + + /** + * Clears the compiled script cache. Useful for testing or when scripts are updated. + */ + public static void clearCompiledScriptCache() { + COMPILED_SCRIPT_CACHE.clear(); + SCRIPT_COMPILE_COUNT.clear(); + SCRIPT_CACHE_HIT_COUNT.clear(); + } + + /** + * Gets cache statistics for monitoring. + * @return map with "cacheSize", "totalCompiles", "totalCacheHits" + */ + public static Map getCacheStatistics() { + long totalCompiles = SCRIPT_COMPILE_COUNT.values().stream().mapToLong(Long::longValue).sum(); + long totalHits = SCRIPT_CACHE_HIT_COUNT.values().stream().mapToLong(Long::longValue).sum(); + return Map.of( + "cacheSize", (long) COMPILED_SCRIPT_CACHE.size(), + "totalCompiles", totalCompiles, + "totalCacheHits", totalHits + ); + } /** * @@ -56,30 +91,103 @@ public ScriptIOBean runScript(@Nonnull String script, @Nonnull ScriptEngine engi } /** + * Runs a script with compiled script caching for performance. * - * @param scriptName - * @param script - * @param engine - * @param inputs - * @param output - * @return - * @throws ScriptException + * + * @param scriptName unique name for script caching (null disables caching) + * @param script the script source code + * @param engine the ScriptEngine to use + * @param inputs input variables for the script + * @param output output logger + * @return ScriptIOBean with execution results + * @throws ScriptException if script execution fails */ public ScriptIOBean runScript(@Nullable String scriptName, @Nonnull String script, @Nonnull ScriptEngine engine, @Nonnull Map inputs, OutputLogger output) throws ScriptException { - ScriptIOBean ioBean = null; - try ( Reader reader = new StringReader(script) ){ - ioBean = new ScriptIOBean(inputs, output); - engine.put("ioBean", ioBean); - ioBean.debug("Starting scriptEngine..."); - engine.eval(reader, engine.getContext()); + ScriptIOBean ioBean = new ScriptIOBean(inputs, output); + ioBean.debug("Starting scriptEngine..."); + + try { + // Create isolated ScriptContext to prevent variable pollution between executions + ScriptContext context = new SimpleScriptContext(); + context.setBindings(engine.createBindings(), ScriptContext.ENGINE_SCOPE); + + // Add ioBean to context + context.getBindings(ScriptContext.ENGINE_SCOPE).put("ioBean", ioBean); + + // Copy input variables to context + for (Map.Entry entry : inputs.entrySet()) { + context.getBindings(ScriptContext.ENGINE_SCOPE).put(entry.getKey(), entry.getValue()); + } + + // Use compiled script caching if possible + if (scriptName != null && !scriptName.isEmpty() && engine instanceof Compilable) { + executeCompiledScript(scriptName, script, (Compilable) engine, context); + } else { + // Fallback: direct evaluation (no caching) + try (Reader reader = new StringReader(script)) { + engine.eval(reader, context); + } + } + ioBean.debug("Finished scriptEngine..."); } catch (ScriptException e) { throw new ScriptException(e.getMessage(), scriptName, e.getLineNumber(), e.getColumnNumber()); } catch (IOException e) { throw new ScriptException(e.getMessage(), scriptName, 0, 0); } + return ioBean; } + + /** + * Executes a script using compiled script caching. + * + * @param scriptName unique script identifier for caching + * @param script source code to compile (if not cached) + * @param compilable the Compilable engine + * @param context the ScriptContext to execute in + * @throws ScriptException if compilation or execution fails + */ + private void executeCompiledScript(@Nonnull String scriptName, @Nonnull String script, + @Nonnull Compilable compilable, @Nonnull ScriptContext context) + throws ScriptException { + CompiledScript compiledScript = COMPILED_SCRIPT_CACHE.get(scriptName); + + if (compiledScript == null) { + // Cache miss: compile and cache the script + compiledScript = compileAndCache(scriptName, script, compilable); + SCRIPT_COMPILE_COUNT.merge(scriptName, 1L, Long::sum); + } else { + // Cache hit: use cached compiled script + SCRIPT_CACHE_HIT_COUNT.merge(scriptName, 1L, Long::sum); + } + + // Execute the compiled script with isolated context + compiledScript.eval(context); + } + + /** + * Compiles a script and caches it. + * Thread-safe: uses computeIfAbsent to handle race conditions. + * + * @param scriptName unique script identifier + * @param script source code to compile + * @param compilable the Compilable engine + * @return compiled script + * @throws ScriptException if compilation fails + */ + private CompiledScript compileAndCache(@Nonnull String scriptName, @Nonnull String script, + @Nonnull Compilable compilable) throws ScriptException { + // Use computeIfAbsent to handle race conditions (only one thread compiles) + return COMPILED_SCRIPT_CACHE.computeIfAbsent(scriptName, name -> { + try { + return compilable.compile(script); + } catch (ScriptException e) { + // Wrap in RuntimeException since computeIfAbsent doesn't allow checked exceptions + throw new RuntimeException("Failed to compile script: " + name, e); + } + }); + } }