Merge pull request #1300 from HubSpot/jasmith_hubspot/jinjava-3.0-perf

Performance optimizations for jinjava 3.0 expression resolution

Author: jasmith-hs

src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java

@@ -284,6 +284,9 @@ Object wrap(Object value) {
     if (value == null) {
       return null;
     }
+    if (value instanceof String || value instanceof Number || value instanceof Boolean) {
+      return value;
+    }
     if (value instanceof PyishSerializable) {
       return value;
     }

src/main/java/com/hubspot/jinjava/el/ext/AllowlistReturnTypeValidator.java

@@ -11,7 +11,7 @@ public final class AllowlistReturnTypeValidator {
     AllowlistReturnTypeValidator.create(
       ReturnTypeValidatorConfig.builder().addDefaultAllowlistGroups().build()
     );
-  private final ConcurrentHashMap<String, Boolean> allowedReturnTypesCache;
+  private final ConcurrentHashMap<Class<?>, Boolean> allowedReturnTypesCache;
 
   private final ImmutableSet<String> allowedCanonicalClassPrefixes;
   private final ImmutableSet<String> allowedCanonicalClassNames;
@@ -47,20 +47,25 @@ public Object validateReturnType(Object o) {
     if (o == null) {
       return null;
     }
+    if (o instanceof String || o instanceof Number || o instanceof Boolean) {
+      return o;
+    }
     Class<?> clazz = o.getClass();
     if (clazz.isArray() && allowArrays) {
       return o;
     }
-    String canonicalClassName = clazz.getCanonicalName();
-    if (canonicalClassName == null) {
-      onRejectedClass.accept(clazz);
-      return null;
-    }
     boolean isAllowedClassName = allowedReturnTypesCache.computeIfAbsent(
-      canonicalClassName,
-      c ->
-        allowedCanonicalClassNames.contains(canonicalClassName) ||
-        allowedCanonicalClassPrefixes.stream().anyMatch(canonicalClassName::startsWith)
+      clazz,
+      c -> {
+        String canonicalClassName = c.getCanonicalName();
+        if (canonicalClassName == null) {
+          return false;
+        }
+        return (
+          allowedCanonicalClassNames.contains(canonicalClassName) ||
+          allowedCanonicalClassPrefixes.stream().anyMatch(canonicalClassName::startsWith)
+        );
+      }
     );
     if (!isAllowedClassName) {
       onRejectedClass.accept(clazz);

src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java

@@ -977,11 +977,11 @@ public List<TemplateError> getErrorsCopy() {
     ThreadLocal.withInitial(Stack::new);
 
   public static JinjavaInterpreter getCurrent() {
-    if (CURRENT_INTERPRETER.get().isEmpty()) {
+    Stack<JinjavaInterpreter> stack = CURRENT_INTERPRETER.get();
+    if (stack.isEmpty()) {
       return null;
     }
-
-    return CURRENT_INTERPRETER.get().peek();
+    return stack.peek();
   }
 
   public static Optional<JinjavaInterpreter> getCurrentMaybe() {

src/main/java/com/hubspot/jinjava/objects/serialization/PyishObjectMapper.java

@@ -57,10 +57,13 @@ private static ObjectMapper getPyishObjectMapper() {
   }
 
   public static String getAsUnquotedPyishString(Object val) {
-    if (val != null) {
-      return WhitespaceUtils.unquoteAndUnescape(getAsPyishString(val, true));
+    if (val == null) {
+      return "";
     }
-    return "";
+    if (val instanceof String || val instanceof Number || val instanceof Boolean) {
+      return val.toString();
+    }
+    return WhitespaceUtils.unquoteAndUnescape(getAsPyishString(val, true));
   }
 
   public static String getAsPyishString(Object val) {

src/main/java/com/hubspot/jinjava/util/ScopeMap.java

@@ -1,16 +1,12 @@
 package com.hubspot.jinjava.util;
 
-import static com.hubspot.jinjava.util.Logging.ENGINE_LOG;
-
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
-import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 
 public class ScopeMap<K, V> implements Map<K, V> {
@@ -25,25 +21,6 @@ public ScopeMap() {
   public ScopeMap(ScopeMap<K, V> parent) {
     this.scope = new HashMap<>();
     this.parent = parent;
-
-    Set<ScopeMap<K, V>> parents = new HashSet<>();
-    if (parent != null) {
-      ScopeMap<K, V> p = parent.getParent();
-      while (p != null) {
-        parents.add(p);
-        if (parents.contains(parent)) {
-          ENGINE_LOG.error(
-            "Parent loop detected:\n{}",
-            Arrays
-              .stream(Thread.currentThread().getStackTrace())
-              .map(StackTraceElement::toString)
-              .collect(Collectors.joining("\n"))
-          );
-          break;
-        }
-        p = p.getParent();
-      }
-    }
   }
 
   public ScopeMap(ScopeMap<K, V> parent, Map<K, V> scope) {