From c3684375da87ae72d952f7aa8ccbe164b686abae Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 8 Jan 2018 13:59:27 -0800 Subject: [PATCH 1/3] Added cache. --- .../elasticsearch/painless/Definition.java | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java index 34385dbf7acfc..587fba12977b8 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java @@ -41,6 +41,9 @@ */ public final class Definition { + private static final Map methodCache = new HashMap<>(); + private static final Map fieldCache = new HashMap<>(); + private static final Pattern TYPE_NAME_PATTERN = Pattern.compile("^[_a-zA-Z][._a-zA-Z0-9]*$"); public static final String[] DEFINITION_FILES = new String[] { @@ -533,6 +536,43 @@ Collection allSimpleTypes() { return simpleTypesMap.values(); } + private static Method putMethodInCache(Method method) { + StringBuilder key = new StringBuilder(); + key.append(method.owner.name); + key.append(method.name); + + for (Type argument : method.arguments) { + key.append(argument.name); + } + + Method cached = methodCache.get(key.toString()); + + if (cached == null) { + methodCache.put(key.toString(), method); + + return method; + } else { + return cached; + } + } + + private static Field putFieldInCache(Field field) { + StringBuilder key = new StringBuilder(); + key.append(field.owner.name); + key.append(field.name); + key.append(field.type.name); + + Field cached = fieldCache.get(key.toString()); + + if (cached == null) { + fieldCache.put(key.toString(), field); + + return field; + } else { + return cached; + } + } + // INTERNAL IMPLEMENTATION: private final Map, RuntimeClass> runtimeMap; @@ -941,7 +981,7 @@ private void addMethod(ClassLoader whitelistClassLoader, String ownerStructName, painlessMethod = new Method(whitelistMethod.javaMethodName, ownerStruct, null, painlessReturnType, painlessParametersTypes, asmMethod, javaMethod.getModifiers(), javaMethodHandle); - ownerStruct.staticMethods.put(painlessMethodKey, painlessMethod); + ownerStruct.staticMethods.put(painlessMethodKey, putMethodInCache(painlessMethod)); } else if ((painlessMethod.name.equals(whitelistMethod.javaMethodName) && painlessMethod.rtn.equals(painlessReturnType) && painlessMethod.arguments.equals(painlessParametersTypes)) == false) { throw new IllegalArgumentException("illegal duplicate static methods [" + painlessMethodKey + "] " + @@ -965,7 +1005,7 @@ private void addMethod(ClassLoader whitelistClassLoader, String ownerStructName, painlessMethod = new Method(whitelistMethod.javaMethodName, ownerStruct, javaAugmentedClass, painlessReturnType, painlessParametersTypes, asmMethod, javaMethod.getModifiers(), javaMethodHandle); - ownerStruct.methods.put(painlessMethodKey, painlessMethod); + ownerStruct.methods.put(painlessMethodKey, putMethodInCache(painlessMethod)); } else if ((painlessMethod.name.equals(whitelistMethod.javaMethodName) && painlessMethod.rtn.equals(painlessReturnType) && painlessMethod.arguments.equals(painlessParametersTypes)) == false) { throw new IllegalArgumentException("illegal duplicate member methods [" + painlessMethodKey + "] " + @@ -1018,7 +1058,7 @@ private void addField(String ownerStructName, Whitelist.Field whitelistField) { if (painlessField == null) { painlessField = new Field(whitelistField.javaFieldName, javaField.getName(), ownerStruct, painlessFieldType, javaField.getModifiers(), null, null); - ownerStruct.staticMembers.put(whitelistField.javaFieldName, painlessField); + ownerStruct.staticMembers.put(whitelistField.javaFieldName, putFieldInCache(painlessField)); } else if (painlessField.type.equals(painlessFieldType) == false) { throw new IllegalArgumentException("illegal duplicate static fields [" + whitelistField.javaFieldName + "] " + "found within the struct [" + ownerStruct.name + "] with type [" + whitelistField.painlessFieldTypeName + "]"); @@ -1042,7 +1082,7 @@ private void addField(String ownerStructName, Whitelist.Field whitelistField) { if (painlessField == null) { painlessField = new Field(whitelistField.javaFieldName, javaField.getName(), ownerStruct, painlessFieldType, javaField.getModifiers(), javaMethodHandleGetter, javaMethodHandleSetter); - ownerStruct.staticMembers.put(whitelistField.javaFieldName, painlessField); + ownerStruct.staticMembers.put(whitelistField.javaFieldName, putFieldInCache(painlessField)); } else if (painlessField.type.equals(painlessFieldType) == false) { throw new IllegalArgumentException("illegal duplicate member fields [" + whitelistField.javaFieldName + "] " + "found within the struct [" + ownerStruct.name + "] with type [" + whitelistField.painlessFieldTypeName + "]"); From b895ff0f867e80aaf1714db696500acdaac1149e Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 8 Jan 2018 16:03:26 -0800 Subject: [PATCH 2/3] Add constructor cache. --- .../src/main/java/org/elasticsearch/painless/Definition.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java index 587fba12977b8..65d1463e1e888 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java @@ -878,7 +878,7 @@ private void addConstructor(String ownerStructName, Whitelist.Constructor whitel painlessConstructor = new Method("", ownerStruct, null, getTypeInternal("void"), painlessParametersTypes, asmConstructor, javaConstructor.getModifiers(), javaHandle); - ownerStruct.constructors.put(painlessMethodKey, painlessConstructor); + ownerStruct.constructors.put(painlessMethodKey, putMethodInCache(painlessConstructor)); } else if (painlessConstructor.arguments.equals(painlessParametersTypes) == false){ throw new IllegalArgumentException( "illegal duplicate constructors [" + painlessMethodKey + "] found within the struct [" + ownerStruct.name + "] " + From 9bf62f554783d4c39d53069a965e001e1289fca4 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Mon, 8 Jan 2018 16:46:25 -0800 Subject: [PATCH 3/3] Cleaned up code. --- .../elasticsearch/painless/Definition.java | 88 +++++++++---------- 1 file changed, 41 insertions(+), 47 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java index 65d1463e1e888..7d8b4ff4e614e 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java @@ -536,41 +536,20 @@ Collection allSimpleTypes() { return simpleTypesMap.values(); } - private static Method putMethodInCache(Method method) { + private static String buildMethodCacheKey(String structName, String methodName, List arguments) { StringBuilder key = new StringBuilder(); - key.append(method.owner.name); - key.append(method.name); + key.append(structName); + key.append(methodName); - for (Type argument : method.arguments) { + for (Type argument : arguments) { key.append(argument.name); } - Method cached = methodCache.get(key.toString()); - - if (cached == null) { - methodCache.put(key.toString(), method); - - return method; - } else { - return cached; - } + return key.toString(); } - private static Field putFieldInCache(Field field) { - StringBuilder key = new StringBuilder(); - key.append(field.owner.name); - key.append(field.name); - key.append(field.type.name); - - Field cached = fieldCache.get(key.toString()); - - if (cached == null) { - fieldCache.put(key.toString(), field); - - return field; - } else { - return cached; - } + private static String buildFieldCacheKey(String structName, String fieldName, String typeName) { + return structName + fieldName + typeName; } // INTERNAL IMPLEMENTATION: @@ -876,9 +855,11 @@ private void addConstructor(String ownerStructName, Whitelist.Constructor whitel " with constructor parameters " + whitelistConstructor.painlessParameterTypeNames); } - painlessConstructor = new Method("", ownerStruct, null, getTypeInternal("void"), painlessParametersTypes, - asmConstructor, javaConstructor.getModifiers(), javaHandle); - ownerStruct.constructors.put(painlessMethodKey, putMethodInCache(painlessConstructor)); + painlessConstructor = methodCache.computeIfAbsent(buildMethodCacheKey(ownerStruct.name, "", painlessParametersTypes), + key -> new Method("", ownerStruct, null, getTypeInternal("void"), painlessParametersTypes, + asmConstructor, javaConstructor.getModifiers(), javaHandle)); + + ownerStruct.constructors.put(painlessMethodKey, painlessConstructor); } else if (painlessConstructor.arguments.equals(painlessParametersTypes) == false){ throw new IllegalArgumentException( "illegal duplicate constructors [" + painlessMethodKey + "] found within the struct [" + ownerStruct.name + "] " + @@ -899,7 +880,7 @@ private void addMethod(ClassLoader whitelistClassLoader, String ownerStructName, " [" + whitelistMethod.javaMethodName + "] for owner struct [" + ownerStructName + "]."); } - Class javaAugmentedClass = null; + Class javaAugmentedClass; if (whitelistMethod.javaAugmentedClassName != null) { try { @@ -909,6 +890,8 @@ private void addMethod(ClassLoader whitelistClassLoader, String ownerStructName, "not found for method with name [" + whitelistMethod.javaMethodName + "] " + "and parameters " + whitelistMethod.painlessParameterTypeNames, cnfe); } + } else { + javaAugmentedClass = null; } int augmentedOffset = javaAugmentedClass == null ? 0 : 1; @@ -979,9 +962,11 @@ private void addMethod(ClassLoader whitelistClassLoader, String ownerStructName, "[" + whitelistMethod.javaMethodName + "] and parameters " + whitelistMethod.painlessParameterTypeNames); } - painlessMethod = new Method(whitelistMethod.javaMethodName, ownerStruct, null, painlessReturnType, - painlessParametersTypes, asmMethod, javaMethod.getModifiers(), javaMethodHandle); - ownerStruct.staticMethods.put(painlessMethodKey, putMethodInCache(painlessMethod)); + painlessMethod = methodCache.computeIfAbsent( + buildMethodCacheKey(ownerStruct.name, whitelistMethod.javaMethodName, painlessParametersTypes), + key -> new Method(whitelistMethod.javaMethodName, ownerStruct, null, painlessReturnType, painlessParametersTypes, + asmMethod, javaMethod.getModifiers(), javaMethodHandle)); + ownerStruct.staticMethods.put(painlessMethodKey, painlessMethod); } else if ((painlessMethod.name.equals(whitelistMethod.javaMethodName) && painlessMethod.rtn.equals(painlessReturnType) && painlessMethod.arguments.equals(painlessParametersTypes)) == false) { throw new IllegalArgumentException("illegal duplicate static methods [" + painlessMethodKey + "] " + @@ -1003,9 +988,11 @@ private void addMethod(ClassLoader whitelistClassLoader, String ownerStructName, "[" + whitelistMethod.javaMethodName + "] and parameters " + whitelistMethod.painlessParameterTypeNames); } - painlessMethod = new Method(whitelistMethod.javaMethodName, ownerStruct, javaAugmentedClass, painlessReturnType, - painlessParametersTypes, asmMethod, javaMethod.getModifiers(), javaMethodHandle); - ownerStruct.methods.put(painlessMethodKey, putMethodInCache(painlessMethod)); + painlessMethod = methodCache.computeIfAbsent( + buildMethodCacheKey(ownerStruct.name, whitelistMethod.javaMethodName, painlessParametersTypes), + key -> new Method(whitelistMethod.javaMethodName, ownerStruct, javaAugmentedClass, painlessReturnType, + painlessParametersTypes, asmMethod, javaMethod.getModifiers(), javaMethodHandle)); + ownerStruct.methods.put(painlessMethodKey, painlessMethod); } else if ((painlessMethod.name.equals(whitelistMethod.javaMethodName) && painlessMethod.rtn.equals(painlessReturnType) && painlessMethod.arguments.equals(painlessParametersTypes)) == false) { throw new IllegalArgumentException("illegal duplicate member methods [" + painlessMethodKey + "] " + @@ -1056,33 +1043,40 @@ private void addField(String ownerStructName, Whitelist.Field whitelistField) { Field painlessField = ownerStruct.staticMembers.get(whitelistField.javaFieldName); if (painlessField == null) { - painlessField = new Field(whitelistField.javaFieldName, javaField.getName(), - ownerStruct, painlessFieldType, javaField.getModifiers(), null, null); - ownerStruct.staticMembers.put(whitelistField.javaFieldName, putFieldInCache(painlessField)); + painlessField = fieldCache.computeIfAbsent( + buildFieldCacheKey(ownerStruct.name, whitelistField.javaFieldName, painlessFieldType.name), + key -> new Field(whitelistField.javaFieldName, javaField.getName(), + ownerStruct, painlessFieldType, javaField.getModifiers(), null, null)); + ownerStruct.staticMembers.put(whitelistField.javaFieldName, painlessField); } else if (painlessField.type.equals(painlessFieldType) == false) { throw new IllegalArgumentException("illegal duplicate static fields [" + whitelistField.javaFieldName + "] " + "found within the struct [" + ownerStruct.name + "] with type [" + whitelistField.painlessFieldTypeName + "]"); } } else { - MethodHandle javaMethodHandleGetter = null; - MethodHandle javaMethodHandleSetter = null; + MethodHandle javaMethodHandleGetter; + MethodHandle javaMethodHandleSetter; try { if (Modifier.isStatic(javaField.getModifiers()) == false) { javaMethodHandleGetter = MethodHandles.publicLookup().unreflectGetter(javaField); javaMethodHandleSetter = MethodHandles.publicLookup().unreflectSetter(javaField); + } else { + javaMethodHandleGetter = null; + javaMethodHandleSetter = null; } } catch (IllegalAccessException exception) { throw new IllegalArgumentException("getter/setter [" + whitelistField.javaFieldName + "]" + " not found for class [" + ownerStruct.clazz.getName() + "]."); } - Field painlessField = ownerStruct.staticMembers.get(whitelistField.javaFieldName); + Field painlessField = ownerStruct.members.get(whitelistField.javaFieldName); if (painlessField == null) { - painlessField = new Field(whitelistField.javaFieldName, javaField.getName(), - ownerStruct, painlessFieldType, javaField.getModifiers(), javaMethodHandleGetter, javaMethodHandleSetter); - ownerStruct.staticMembers.put(whitelistField.javaFieldName, putFieldInCache(painlessField)); + painlessField = fieldCache.computeIfAbsent( + buildFieldCacheKey(ownerStruct.name, whitelistField.javaFieldName, painlessFieldType.name), + key -> new Field(whitelistField.javaFieldName, javaField.getName(), + ownerStruct, painlessFieldType, javaField.getModifiers(), javaMethodHandleGetter, javaMethodHandleSetter)); + ownerStruct.members.put(whitelistField.javaFieldName, painlessField); } else if (painlessField.type.equals(painlessFieldType) == false) { throw new IllegalArgumentException("illegal duplicate member fields [" + whitelistField.javaFieldName + "] " + "found within the struct [" + ownerStruct.name + "] with type [" + whitelistField.painlessFieldTypeName + "]");