From dee81fea2653ab22aa9ae97c7b9dd33c1cb0b60a Mon Sep 17 00:00:00 2001 From: ViacheslavKlimov Date: Mon, 14 Nov 2022 18:00:19 +0200 Subject: [PATCH] MVEL scripts caching --- .../src/main/resources/thingsboard.yml | 1 + .../service/script/MvelInvokeServiceTest.java | 98 +++++++++++++++++++ common/script/script-api/pom.xml | 4 + .../api/mvel/DefaultMvelInvokeService.java | 65 ++++++++++-- .../script/api/mvel/MvelScript.java | 1 - 5 files changed, 158 insertions(+), 11 deletions(-) diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index 0f110da90c..cd05c7ff23 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -625,6 +625,7 @@ mvel: max_black_list_duration_sec: "${MVEL_MAX_BLACKLIST_DURATION_SEC:60}" # Specify thread pool size for javascript executor service thread_pool_size: "${MVEL_THREAD_POOL_SIZE:50}" + compiled_scripts_cache_size: "${MVEL_COMPILED_SCRIPTS_CACHE_SIZE:2000}" stats: enabled: "${TB_MVEL_STATS_ENABLED:false}" print_interval_ms: "${TB_MVEL_STATS_PRINT_INTERVAL_MS:10000}" diff --git a/application/src/test/java/org/thingsboard/server/service/script/MvelInvokeServiceTest.java b/application/src/test/java/org/thingsboard/server/service/script/MvelInvokeServiceTest.java index a40a087514..1c3c134a82 100644 --- a/application/src/test/java/org/thingsboard/server/service/script/MvelInvokeServiceTest.java +++ b/application/src/test/java/org/thingsboard/server/service/script/MvelInvokeServiceTest.java @@ -16,6 +16,7 @@ package org.thingsboard.server.service.script; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.github.benmanes.caffeine.cache.Cache; import org.junit.Assert; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -24,15 +25,22 @@ import org.springframework.test.context.TestPropertySource; import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.script.api.ScriptType; import org.thingsboard.script.api.mvel.MvelInvokeService; +import org.thingsboard.script.api.mvel.MvelScript; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.controller.AbstractControllerTest; import org.thingsboard.server.dao.service.DaoSqlTest; +import java.io.Serializable; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @DaoSqlTest @@ -41,6 +49,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; "mvel.max_total_args_size=50", "mvel.max_result_size=50", "mvel.max_errors=2", + "mvel.compiled_scripts_cache_size=100" }) class MvelInvokeServiceTest extends AbstractControllerTest { @@ -110,6 +119,89 @@ class MvelInvokeServiceTest extends AbstractControllerTest { assertThatScriptIsBlocked(scriptId); } + @Test + void givenScriptsWithSameBody_thenCompileAndCacheOnlyOnce() throws Exception { + String script = "return msg.temperature > 20;"; + List scriptsIds = new ArrayList<>(); + for (int i = 0; i < 100; i++) { + UUID scriptId = evalScript(script); + scriptsIds.add(scriptId); + } + + Map scriptIdToHash = getFieldValue(invokeService, "scriptIdToHash"); + Map scriptMap = getFieldValue(invokeService, "scriptMap"); + Cache compiledScriptsCache = getFieldValue(invokeService, "compiledScriptsCache"); + + String scriptHash = scriptIdToHash.get(scriptsIds.get(0)); + + assertThat(scriptsIds.stream().map(scriptIdToHash::get)).containsOnly(scriptHash); + assertThat(scriptMap).containsKey(scriptHash); + assertThat(compiledScriptsCache.getIfPresent(scriptHash)).isNotNull(); + } + + @Test + public void whenReleasingScript_thenCheckForScriptHashUsages() throws Exception { + String script = "return msg.temperature > 20;"; + List scriptsIds = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + UUID scriptId = evalScript(script); + scriptsIds.add(scriptId); + } + + Map scriptIdToHash = getFieldValue(invokeService, "scriptIdToHash"); + Map scriptMap = getFieldValue(invokeService, "scriptMap"); + Cache compiledScriptsCache = getFieldValue(invokeService, "compiledScriptsCache"); + + String scriptHash = scriptIdToHash.get(scriptsIds.get(0)); + for (int i = 0; i < 9; i++) { + UUID scriptId = scriptsIds.get(i); + assertThat(scriptIdToHash).containsKey(scriptId); + invokeService.release(scriptId); + assertThat(scriptIdToHash).doesNotContainKey(scriptId); + } + assertThat(scriptMap).containsKey(scriptHash); + assertThat(compiledScriptsCache.getIfPresent(scriptHash)).isNotNull(); + + invokeService.release(scriptsIds.get(9)); + assertThat(scriptMap).doesNotContainKey(scriptHash); + assertThat(compiledScriptsCache.getIfPresent(scriptHash)).isNull(); + } + + @Test + public void whenCompiledScriptsCacheIsTooBig_thenRemoveRarelyUsedScripts() throws Exception { + Map scriptIdToHash = getFieldValue(invokeService, "scriptIdToHash"); + Cache compiledScriptsCache = getFieldValue(invokeService, "compiledScriptsCache"); + + List scriptsIds = new ArrayList<>(); + for (int i = 0; i < 110; i++) { // mvel.compiled_scripts_cache_size = 100 + String script = "return msg.temperature > " + i; + UUID scriptId = evalScript(script); + scriptsIds.add(scriptId); + + for (int j = 0; j < i; j++) { + invokeScript(scriptId, "{ \"temperature\": 12 }"); // so that scriptsIds is ordered by number of invocations + } + } + + ConcurrentMap cache = compiledScriptsCache.asMap(); + + for (int i = 0; i < 10; i++) { // iterating rarely used scripts + UUID scriptId = scriptsIds.get(i); + String scriptHash = scriptIdToHash.get(scriptId); + assertThat(cache).doesNotContainKey(scriptHash); + } + for (int i = 10; i < 110; i++) { + UUID scriptId = scriptsIds.get(i); + String scriptHash = scriptIdToHash.get(scriptId); + assertThat(cache).containsKey(scriptHash); + } + + UUID scriptRemovedFromCache = scriptsIds.get(0); + assertThat(compiledScriptsCache.getIfPresent(scriptIdToHash.get(scriptRemovedFromCache))).isNull(); + invokeScript(scriptRemovedFromCache, "{ \"temperature\": 12 }"); + assertThat(compiledScriptsCache.getIfPresent(scriptIdToHash.get(scriptRemovedFromCache))).isNotNull(); + } + private void assertThatScriptIsBlocked(UUID scriptId) { assertThatThrownBy(() -> { invokeScript(scriptId, "{}"); @@ -125,4 +217,10 @@ class MvelInvokeServiceTest extends AbstractControllerTest { return invokeService.invokeScript(TenantId.SYS_TENANT_ID, null, scriptId, msg, "{}", "POST_TELEMETRY_REQUEST").get().toString(); } + private T getFieldValue(Object target, String fieldName) throws Exception { + Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + return (T) field.get(target); + } + } diff --git a/common/script/script-api/pom.xml b/common/script/script-api/pom.xml index 62dc00ca97..b7ae4993f9 100644 --- a/common/script/script-api/pom.xml +++ b/common/script/script-api/pom.xml @@ -56,6 +56,10 @@ com.google.code.gson gson + + com.github.ben-manes.caffeine + caffeine + org.slf4j slf4j-api diff --git a/common/script/script-api/src/main/java/org/thingsboard/script/api/mvel/DefaultMvelInvokeService.java b/common/script/script-api/src/main/java/org/thingsboard/script/api/mvel/DefaultMvelInvokeService.java index 901a49180e..120be2af4f 100644 --- a/common/script/script-api/src/main/java/org/thingsboard/script/api/mvel/DefaultMvelInvokeService.java +++ b/common/script/script-api/src/main/java/org/thingsboard/script/api/mvel/DefaultMvelInvokeService.java @@ -15,6 +15,10 @@ */ package org.thingsboard.script.api.mvel; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.google.common.hash.Hasher; +import com.google.common.hash.Hashing; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; @@ -42,6 +46,7 @@ import org.thingsboard.server.common.stats.TbApiUsageStateClient; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; import java.io.Serializable; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.Map; import java.util.Optional; @@ -55,7 +60,10 @@ import java.util.regex.Pattern; @Service public class DefaultMvelInvokeService extends AbstractScriptInvokeService implements MvelInvokeService { - protected Map scriptMap = new ConcurrentHashMap<>(); + protected final Map scriptIdToHash = new ConcurrentHashMap<>(); + protected final Map scriptMap = new ConcurrentHashMap<>(); + protected Cache compiledScriptsCache; + private SandboxedParserConfiguration parserConfig; private static final Pattern NEW_KEYWORD_PATTERN = Pattern.compile("new\\s"); @@ -92,6 +100,9 @@ public class DefaultMvelInvokeService extends AbstractScriptInvokeService implem @Value("${mvel.max_memory_limit_mb:8}") private long maxMemoryLimitMb; + @Value("${mvel.compiled_scripts_cache_size:2000}") + private int compiledScriptsCacheSize; + private ListeningExecutorService executor; protected DefaultMvelInvokeService(Optional apiUsageStateClient, Optional apiUsageReportClient) { @@ -115,11 +126,14 @@ public class DefaultMvelInvokeService extends AbstractScriptInvokeService implem executor = MoreExecutors.listeningDecorator(ThingsBoardExecutors.newWorkStealingPool(threadPoolSize, "mvel-executor")); try { // Special command to warm up MVEL engine - Serializable script = MVEL.compileExpression("var warmUp = {}; warmUp", new SandboxedParserContext(parserConfig)); + Serializable script = compileScript("var warmUp = {}; warmUp"); MVEL.executeTbExpression(script, new ExecutionContext(parserConfig), Collections.emptyMap()); } catch (Exception e) { // do nothing } + compiledScriptsCache = Caffeine.newBuilder() + .maximumSize(compiledScriptsCacheSize) + .build(); } @PreDestroy @@ -141,16 +155,21 @@ public class DefaultMvelInvokeService extends AbstractScriptInvokeService implem @Override protected boolean isScriptPresent(UUID scriptId) { - return scriptMap.containsKey(scriptId); + return scriptIdToHash.containsKey(scriptId); } @Override protected ListenableFuture doEvalScript(TenantId tenantId, ScriptType scriptType, String scriptBody, UUID scriptId, String[] argNames) { return executor.submit(() -> { try { - Serializable compiledScript = MVEL.compileExpression(scriptBody, new SandboxedParserContext(parserConfig)); - MvelScript script = new MvelScript(compiledScript, scriptBody, argNames); - scriptMap.put(scriptId, script); + String scriptHash = hash(scriptBody, argNames); + compiledScriptsCache.get(scriptHash, k -> { + return compileScript(scriptBody); + }); + scriptIdToHash.put(scriptId, scriptHash); + scriptMap.computeIfAbsent(scriptHash, k -> { + return new MvelScript(scriptBody, argNames); + }); return scriptId; } catch (Exception e) { throw new TbScriptException(scriptId, TbScriptException.ErrorCode.COMPILATION, scriptBody, e); @@ -162,12 +181,16 @@ public class DefaultMvelInvokeService extends AbstractScriptInvokeService implem protected MvelScriptExecutionTask doInvokeFunction(UUID scriptId, Object[] args) { ExecutionContext executionContext = new ExecutionContext(this.parserConfig, maxMemoryLimitMb * 1024 * 1024); return new MvelScriptExecutionTask(executionContext, executor.submit(() -> { - MvelScript script = scriptMap.get(scriptId); - if (script == null) { + String scriptHash = scriptIdToHash.get(scriptId); + if (scriptHash == null) { throw new TbScriptException(scriptId, TbScriptException.ErrorCode.OTHER, null, new RuntimeException("Script not found!")); } + MvelScript script = scriptMap.get(scriptHash); + Serializable compiledScript = compiledScriptsCache.get(scriptHash, k -> { + return compileScript(script.getScriptBody()); + }); try { - return MVEL.executeTbExpression(script.getCompiledScript(), executionContext, script.createVars(args)); + return MVEL.executeTbExpression(compiledScript, executionContext, script.createVars(args)); } catch (ScriptMemoryOverflowException e) { throw new TbScriptException(scriptId, TbScriptException.ErrorCode.OTHER, script.getScriptBody(), new RuntimeException("Script memory overflow!")); } catch (Exception e) { @@ -178,6 +201,28 @@ public class DefaultMvelInvokeService extends AbstractScriptInvokeService implem @Override protected void doRelease(UUID scriptId) throws Exception { - scriptMap.remove(scriptId); + String scriptHash = scriptIdToHash.remove(scriptId); + if (scriptHash != null) { + if (scriptIdToHash.containsValue(scriptHash)) { + return; + } + scriptMap.remove(scriptHash); + compiledScriptsCache.invalidate(scriptHash); + } } + + private Serializable compileScript(String scriptBody) { + return MVEL.compileExpression(scriptBody, new SandboxedParserContext(parserConfig)); + } + + @SuppressWarnings("UnstableApiUsage") + protected String hash(String scriptBody, String[] argNames) { + Hasher hasher = Hashing.murmur3_128().newHasher(); + hasher.putUnencodedChars(scriptBody); + for (String argName : argNames) { + hasher.putString(argName, StandardCharsets.UTF_8); + } + return hasher.hash().toString(); + } + } diff --git a/common/script/script-api/src/main/java/org/thingsboard/script/api/mvel/MvelScript.java b/common/script/script-api/src/main/java/org/thingsboard/script/api/mvel/MvelScript.java index 7a84c7b0af..bca5d8d546 100644 --- a/common/script/script-api/src/main/java/org/thingsboard/script/api/mvel/MvelScript.java +++ b/common/script/script-api/src/main/java/org/thingsboard/script/api/mvel/MvelScript.java @@ -24,7 +24,6 @@ import java.util.Map; @Data public class MvelScript { - private final Serializable compiledScript; private final String scriptBody; private final String[] argNames;