Browse Source

Lower Painless Static Memory Footprint (#45487)

* Painless generates a ton of duplicate strings and empty `Hashmap` instances wrapped as unmodifiable
* This change brings down the static footprint of Painless on an idle node by 20MB (after running the PMC benchmark against said node)
   * Since we were looking into ways of optimizing for smaller node sizes I think this is a worthwhile optimization
Armin Braun 6 years ago
parent
commit
4b1bf7db78

+ 8 - 9
modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessClass.java

@@ -20,7 +20,6 @@
 package org.elasticsearch.painless.lookup;
 
 import java.lang.invoke.MethodHandle;
-import java.util.Collections;
 import java.util.Map;
 import java.util.Objects;
 
@@ -44,16 +43,16 @@ public final class PainlessClass {
             Map<String, PainlessMethod> runtimeMethods,
             Map<String, MethodHandle> getterMethodHandles, Map<String, MethodHandle> setterMethodHandles) {
 
-        this.constructors = Collections.unmodifiableMap(constructors);
-        this.staticMethods = Collections.unmodifiableMap(staticMethods);
-        this.methods = Collections.unmodifiableMap(methods);
-        this.staticFields = Collections.unmodifiableMap(staticFields);
-        this.fields = Collections.unmodifiableMap(fields);
+        this.constructors = Map.copyOf(constructors);
+        this.staticMethods = Map.copyOf(staticMethods);
+        this.methods = Map.copyOf(methods);
+        this.staticFields = Map.copyOf(staticFields);
+        this.fields = Map.copyOf(fields);
         this.functionalInterfaceMethod = functionalInterfaceMethod;
 
-        this.getterMethodHandles = Collections.unmodifiableMap(getterMethodHandles);
-        this.setterMethodHandles = Collections.unmodifiableMap(setterMethodHandles);
-        this.runtimeMethods = Collections.unmodifiableMap(runtimeMethods);
+        this.getterMethodHandles = Map.copyOf(getterMethodHandles);
+        this.setterMethodHandles = Map.copyOf(setterMethodHandles);
+        this.runtimeMethods = Map.copyOf(runtimeMethods);
     }
 
     @Override

+ 5 - 6
modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookup.java

@@ -20,7 +20,6 @@
 package org.elasticsearch.painless.lookup;
 
 import java.lang.invoke.MethodHandle;
-import java.util.Collections;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -59,12 +58,12 @@ public final class PainlessLookup {
         Objects.requireNonNull(painlessMethodKeysToPainlessInstanceBindings);
 
         this.javaClassNamesToClasses = javaClassNamesToClasses;
-        this.canonicalClassNamesToClasses = Collections.unmodifiableMap(canonicalClassNamesToClasses);
-        this.classesToPainlessClasses = Collections.unmodifiableMap(classesToPainlessClasses);
+        this.canonicalClassNamesToClasses = Map.copyOf(canonicalClassNamesToClasses);
+        this.classesToPainlessClasses = Map.copyOf(classesToPainlessClasses);
 
-        this.painlessMethodKeysToImportedPainlessMethods = Collections.unmodifiableMap(painlessMethodKeysToImportedPainlessMethods);
-        this.painlessMethodKeysToPainlessClassBindings = Collections.unmodifiableMap(painlessMethodKeysToPainlessClassBindings);
-        this.painlessMethodKeysToPainlessInstanceBindings = Collections.unmodifiableMap(painlessMethodKeysToPainlessInstanceBindings);
+        this.painlessMethodKeysToImportedPainlessMethods = Map.copyOf(painlessMethodKeysToImportedPainlessMethods);
+        this.painlessMethodKeysToPainlessClassBindings = Map.copyOf(painlessMethodKeysToPainlessClassBindings);
+        this.painlessMethodKeysToPainlessInstanceBindings = Map.copyOf(painlessMethodKeysToPainlessInstanceBindings);
     }
 
     public Class<?> javaClassNameToClass(String javaClassName) {

+ 20 - 20
modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessLookupBuilder.java

@@ -263,7 +263,7 @@ public final class PainlessLookupBuilder {
         Class<?> existingClass = javaClassNamesToClasses.get(clazz.getName());
 
         if (existingClass == null) {
-            javaClassNamesToClasses.put(clazz.getName(), clazz);
+            javaClassNamesToClasses.put(clazz.getName().intern(), clazz);
         } else if (existingClass != clazz) {
             throw new IllegalArgumentException("class [" + canonicalClassName + "] " +
                     "cannot represent multiple java classes with the same name from different class loaders");
@@ -281,7 +281,7 @@ public final class PainlessLookupBuilder {
         if (existingPainlessClassBuilder == null) {
             PainlessClassBuilder painlessClassBuilder = new PainlessClassBuilder();
 
-            canonicalClassNamesToClasses.put(canonicalClassName, clazz);
+            canonicalClassNamesToClasses.put(canonicalClassName.intern(), clazz);
             classesToPainlessClassBuilders.put(clazz, painlessClassBuilder);
         }
 
@@ -302,7 +302,7 @@ public final class PainlessLookupBuilder {
                                 "inconsistent no_import parameter found for class [" + canonicalClassName + "]");
                     }
 
-                    canonicalClassNamesToClasses.put(importedCanonicalClassName, clazz);
+                    canonicalClassNamesToClasses.put(importedCanonicalClassName.intern(), clazz);
                 }
             } else if (importedClass != clazz) {
                 throw new IllegalArgumentException("imported class [" + importedCanonicalClassName + "] cannot represent multiple " +
@@ -394,7 +394,7 @@ public final class PainlessLookupBuilder {
 
         if (existingPainlessConstructor == null) {
             newPainlessConstructor = painlessConstructorCache.computeIfAbsent(newPainlessConstructor, key -> key);
-            painlessClassBuilder.constructors.put(painlessConstructorKey, newPainlessConstructor);
+            painlessClassBuilder.constructors.put(painlessConstructorKey.intern(), newPainlessConstructor);
         } else if (newPainlessConstructor.equals(existingPainlessConstructor) == false){
             throw new IllegalArgumentException("cannot add constructors with the same arity but are not equivalent for constructors " +
                     "[[" + targetCanonicalClassName + "], " + typesToCanonicalTypeNames(typeParameters) + "] and " +
@@ -568,9 +568,9 @@ public final class PainlessLookupBuilder {
             newPainlessMethod = painlessMethodCache.computeIfAbsent(newPainlessMethod, key -> key);
 
             if (isStatic) {
-                painlessClassBuilder.staticMethods.put(painlessMethodKey, newPainlessMethod);
+                painlessClassBuilder.staticMethods.put(painlessMethodKey.intern(), newPainlessMethod);
             } else {
-                painlessClassBuilder.methods.put(painlessMethodKey, newPainlessMethod);
+                painlessClassBuilder.methods.put(painlessMethodKey.intern(), newPainlessMethod);
             }
         } else if (newPainlessMethod.equals(existingPainlessMethod) == false) {
             throw new IllegalArgumentException("cannot add methods with the same name and arity but are not equivalent for methods " +
@@ -671,7 +671,7 @@ public final class PainlessLookupBuilder {
 
             if (existingPainlessField == null) {
                 newPainlessField = painlessFieldCache.computeIfAbsent(newPainlessField, key -> key);
-                painlessClassBuilder.staticFields.put(painlessFieldKey, newPainlessField);
+                painlessClassBuilder.staticFields.put(painlessFieldKey.intern(), newPainlessField);
             } else if (newPainlessField.equals(existingPainlessField) == false) {
                 throw new IllegalArgumentException("cannot add fields with the same name but are not equivalent for fields " +
                         "[[" + targetCanonicalClassName + "], [" + fieldName + "], [" +
@@ -695,7 +695,7 @@ public final class PainlessLookupBuilder {
 
             if (existingPainlessField == null) {
                 newPainlessField = painlessFieldCache.computeIfAbsent(newPainlessField, key -> key);
-                painlessClassBuilder.fields.put(painlessFieldKey, newPainlessField);
+                painlessClassBuilder.fields.put(painlessFieldKey.intern(), newPainlessField);
             } else if (newPainlessField.equals(existingPainlessField) == false) {
                 throw new IllegalArgumentException("cannot add fields with the same name but are not equivalent for fields " +
                         "[[" + targetCanonicalClassName + "], [" + fieldName + "], [" +
@@ -768,7 +768,7 @@ public final class PainlessLookupBuilder {
         Class<?> existingTargetClass = javaClassNamesToClasses.get(targetClass.getName());
 
         if (existingTargetClass == null) {
-            javaClassNamesToClasses.put(targetClass.getName(), targetClass);
+            javaClassNamesToClasses.put(targetClass.getName().intern(), targetClass);
         } else if (existingTargetClass != targetClass) {
             throw new IllegalArgumentException("class [" + targetCanonicalClassName + "] " +
                     "cannot represent multiple java classes with the same name from different class loaders");
@@ -845,7 +845,7 @@ public final class PainlessLookupBuilder {
 
         if (existingImportedPainlessMethod == null) {
             newImportedPainlessMethod = painlessMethodCache.computeIfAbsent(newImportedPainlessMethod, key -> key);
-            painlessMethodKeysToImportedPainlessMethods.put(painlessMethodKey, newImportedPainlessMethod);
+            painlessMethodKeysToImportedPainlessMethods.put(painlessMethodKey.intern(), newImportedPainlessMethod);
         } else if (newImportedPainlessMethod.equals(existingImportedPainlessMethod) == false) {
             throw new IllegalArgumentException("cannot add imported methods with the same name and arity " +
                     "but do not have equivalent methods " +
@@ -913,7 +913,7 @@ public final class PainlessLookupBuilder {
         Class<?> existingTargetClass = javaClassNamesToClasses.get(targetClass.getName());
 
         if (existingTargetClass == null) {
-            javaClassNamesToClasses.put(targetClass.getName(), targetClass);
+            javaClassNamesToClasses.put(targetClass.getName().intern(), targetClass);
         } else if (existingTargetClass != targetClass) {
             throw new IllegalArgumentException("class [" + targetCanonicalClassName + "] " +
                     "cannot represent multiple java classes with the same name from different class loaders");
@@ -1040,7 +1040,7 @@ public final class PainlessLookupBuilder {
 
         if (existingPainlessClassBinding == null) {
             newPainlessClassBinding = painlessClassBindingCache.computeIfAbsent(newPainlessClassBinding, key -> key);
-            painlessMethodKeysToPainlessClassBindings.put(painlessMethodKey, newPainlessClassBinding);
+            painlessMethodKeysToPainlessClassBindings.put(painlessMethodKey.intern(), newPainlessClassBinding);
         } else if (newPainlessClassBinding.equals(existingPainlessClassBinding) == false) {
             throw new IllegalArgumentException("cannot add class bindings with the same name and arity " +
                     "but do not have equivalent methods " +
@@ -1104,7 +1104,7 @@ public final class PainlessLookupBuilder {
         Class<?> existingTargetClass = javaClassNamesToClasses.get(targetClass.getName());
 
         if (existingTargetClass == null) {
-            javaClassNamesToClasses.put(targetClass.getName(), targetClass);
+            javaClassNamesToClasses.put(targetClass.getName().intern(), targetClass);
         } else if (existingTargetClass != targetClass) {
             throw new IllegalArgumentException("class [" + targetCanonicalClassName + "] " +
                     "cannot represent multiple java classes with the same name from different class loaders");
@@ -1170,7 +1170,7 @@ public final class PainlessLookupBuilder {
 
         if (existingPainlessInstanceBinding == null) {
             newPainlessInstanceBinding = painlessInstanceBindingCache.computeIfAbsent(newPainlessInstanceBinding, key -> key);
-            painlessMethodKeysToPainlessInstanceBindings.put(painlessMethodKey, newPainlessInstanceBinding);
+            painlessMethodKeysToPainlessInstanceBindings.put(painlessMethodKey.intern(), newPainlessInstanceBinding);
         } else if (newPainlessInstanceBinding.equals(existingPainlessInstanceBinding) == false) {
             throw new IllegalArgumentException("cannot add instances bindings with the same name and arity " +
                     "but do not have equivalent methods " +
@@ -1269,7 +1269,7 @@ public final class PainlessLookupBuilder {
 
             if (existingPainlessMethod == null || existingPainlessMethod.targetClass != newPainlessMethod.targetClass &&
                     existingPainlessMethod.targetClass.isAssignableFrom(newPainlessMethod.targetClass)) {
-                targetPainlessClassBuilder.methods.put(painlessMethodKey, newPainlessMethod);
+                targetPainlessClassBuilder.methods.put(painlessMethodKey.intern(), newPainlessMethod);
             }
         }
 
@@ -1281,7 +1281,7 @@ public final class PainlessLookupBuilder {
             if (existingPainlessField == null ||
                     existingPainlessField.javaField.getDeclaringClass() != newPainlessField.javaField.getDeclaringClass() &&
                     existingPainlessField.javaField.getDeclaringClass().isAssignableFrom(newPainlessField.javaField.getDeclaringClass())) {
-                targetPainlessClassBuilder.fields.put(painlessFieldKey, newPainlessField);
+                targetPainlessClassBuilder.fields.put(painlessFieldKey.intern(), newPainlessField);
             }
         }
     }
@@ -1445,14 +1445,14 @@ public final class PainlessLookupBuilder {
                 MethodHandle bridgeHandle = MethodHandles.publicLookup().in(bridgeClass).unreflect(bridgeClass.getMethods()[0]);
                 bridgePainlessMethod = new PainlessMethod(bridgeMethod, bridgeClass,
                         painlessMethod.returnType, bridgeTypeParameters, bridgeHandle, bridgeMethodType);
-                painlessClassBuilder.runtimeMethods.put(painlessMethodKey, bridgePainlessMethod);
+                painlessClassBuilder.runtimeMethods.put(painlessMethodKey.intern(), bridgePainlessMethod);
                 painlessBridgeCache.put(painlessMethod, bridgePainlessMethod);
             } catch (Exception exception) {
                 throw new IllegalStateException(
                         "internal error occurred attempting to generate a bridge method [" + bridgeClassName + "]", exception);
             }
         } else {
-            painlessClassBuilder.runtimeMethods.put(painlessMethodKey, bridgePainlessMethod);
+            painlessClassBuilder.runtimeMethods.put(painlessMethodKey.intern(), bridgePainlessMethod);
         }
     }
 
@@ -1486,8 +1486,8 @@ public final class PainlessLookupBuilder {
         }
 
         for (PainlessField painlessField : painlessClassBuilder.fields.values()) {
-            painlessClassBuilder.getterMethodHandles.put(painlessField.javaField.getName(), painlessField.getterMethodHandle);
-            painlessClassBuilder.setterMethodHandles.put(painlessField.javaField.getName(), painlessField.setterMethodHandle);
+            painlessClassBuilder.getterMethodHandles.put(painlessField.javaField.getName().intern(), painlessField.getterMethodHandle);
+            painlessClassBuilder.setterMethodHandles.put(painlessField.javaField.getName().intern(), painlessField.setterMethodHandle);
         }
     }
 }

+ 1 - 2
modules/lang-painless/src/main/java/org/elasticsearch/painless/lookup/PainlessMethod.java

@@ -22,7 +22,6 @@ package org.elasticsearch.painless.lookup;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodType;
 import java.lang.reflect.Method;
-import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 
@@ -41,7 +40,7 @@ public class PainlessMethod {
         this.javaMethod = javaMethod;
         this.targetClass = targetClass;
         this.returnType = returnType;
-        this.typeParameters = Collections.unmodifiableList(typeParameters);
+        this.typeParameters = List.copyOf(typeParameters);
         this.methodHandle = methodHandle;
         this.methodType = methodType;
     }