FREEMARKER-80: Avoid calling synchonized PropertyDescriptor.getReadMethod() when getting a property in a template. Will see if it matters.
Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/8f787963 Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/8f787963 Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/8f787963 Branch: refs/heads/2.3-gae Commit: 8f787963600b5ebaafd39bbdcc10121d8f82ce9a Parents: c01f1a2 Author: ddekany <[email protected]> Authored: Tue Oct 3 19:53:49 2017 +0200 Committer: ddekany <[email protected]> Committed: Tue Oct 3 19:54:15 2017 +0200 ---------------------------------------------------------------------- .../java/freemarker/ext/beans/BeanModel.java | 33 +++++----- .../freemarker/ext/beans/ClassIntrospector.java | 64 +++++++------------- .../ext/beans/FastPropertyDescriptor.java | 46 ++++++++++++++ 3 files changed, 84 insertions(+), 59 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/8f787963/src/main/java/freemarker/ext/beans/BeanModel.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/beans/BeanModel.java b/src/main/java/freemarker/ext/beans/BeanModel.java index 01d779a..1f885fa 100644 --- a/src/main/java/freemarker/ext/beans/BeanModel.java +++ b/src/main/java/freemarker/ext/beans/BeanModel.java @@ -19,8 +19,6 @@ package freemarker.ext.beans; -import java.beans.IndexedPropertyDescriptor; -import java.beans.PropertyDescriptor; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -63,8 +61,7 @@ import freemarker.template.utility.StringUtil; */ public class BeanModel -implements - TemplateHashModelEx, AdapterTemplateModel, WrapperTemplateModel, TemplateModelWithAPISupport { +implements TemplateHashModelEx, AdapterTemplateModel, WrapperTemplateModel, TemplateModelWithAPISupport { private static final Logger LOG = Logger.getLogger("freemarker.beans"); protected final Object object; protected final BeansWrapper wrapper; @@ -218,22 +215,22 @@ implements } TemplateModel resultModel = UNKNOWN; - if (desc instanceof IndexedPropertyDescriptor) { - IndexedPropertyDescriptor pd = (IndexedPropertyDescriptor) desc; - Method readMethod; - if (!wrapper.getPreferIndexedReadMethod() && (readMethod = pd.getReadMethod()) != null) { - resultModel = wrapper.invokeMethod(object, readMethod, null); - // cachedModel remains null, as we don't cache these + if (desc instanceof FastPropertyDescriptor) { + FastPropertyDescriptor pd = (FastPropertyDescriptor) desc; + Method indexedReadMethod = pd.getIndexedReadMethod(); + if (indexedReadMethod != null) { + if (!wrapper.getPreferIndexedReadMethod() && (pd.getReadMethod()) != null) { + resultModel = wrapper.invokeMethod(object, pd.getReadMethod(), null); + // cachedModel remains null, as we don't cache these + } else { + resultModel = cachedModel = + new SimpleMethodModel(object, indexedReadMethod, + ClassIntrospector.getArgTypes(classInfo, indexedReadMethod), wrapper); + } } else { - Method indexedReadMethod = pd.getIndexedReadMethod(); - resultModel = cachedModel = - new SimpleMethodModel(object, indexedReadMethod, - ClassIntrospector.getArgTypes(classInfo, indexedReadMethod), wrapper); + resultModel = wrapper.invokeMethod(object, pd.getReadMethod(), null); + // cachedModel remains null, as we don't cache these } - } else if (desc instanceof PropertyDescriptor) { - PropertyDescriptor pd = (PropertyDescriptor) desc; - resultModel = wrapper.invokeMethod(object, pd.getReadMethod(), null); - // cachedModel remains null, as we don't cache these } else if (desc instanceof Field) { resultModel = wrapper.wrap(((Field) desc).get(object)); // cachedModel remains null, as we don't cache these http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/8f787963/src/main/java/freemarker/ext/beans/ClassIntrospector.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/beans/ClassIntrospector.java b/src/main/java/freemarker/ext/beans/ClassIntrospector.java index 72e8855..ce8bbd3 100644 --- a/src/main/java/freemarker/ext/beans/ClassIntrospector.java +++ b/src/main/java/freemarker/ext/beans/ClassIntrospector.java @@ -218,7 +218,7 @@ class ClassIntrospector { * Gets the class introspection data from {@link #cache}, automatically creating the cache entry if it's missing. * * @return A {@link Map} where each key is a property/method/field name (or a special {@link Object} key like - * {@link #CONSTRUCTORS_KEY}), each value is a {@link PropertyDescriptor} or {@link Method} or + * {@link #CONSTRUCTORS_KEY}), each value is a {@link FastPropertyDescriptor} or {@link Method} or * {@link OverloadedMethods} or {@link Field} (but better check the source code...). */ Map<Object, Object> get(Class<?> clazz) { @@ -335,8 +335,7 @@ class ClassIntrospector { int mdsSize = mds.size(); IdentityHashMap<Method, Void> argTypesUsedByIndexerPropReaders = null; for (int i = mdsSize - 1; i >= 0; --i) { - final MethodDescriptor md = mds.get(i); - final Method method = getMatchingAccessibleMethod(md.getMethod(), accessibleMethods); + final Method method = getMatchingAccessibleMethod(mds.get(i).getMethod(), accessibleMethods); if (method != null && isAllowedToExpose(method)) { decision.setDefaults(method); if (methodAppearanceFineTuner != null) { @@ -350,7 +349,7 @@ class ClassIntrospector { } PropertyDescriptor propDesc = decision.getExposeAsProperty(); - if (propDesc != null && !(introspData.get(propDesc.getName()) instanceof PropertyDescriptor)) { + if (propDesc != null && !(introspData.get(propDesc.getName()) instanceof FastPropertyDescriptor)) { addPropertyDescriptorToClassIntrospectionData( introspData, propDesc, clazz, accessibleMethods); } @@ -373,7 +372,7 @@ class ClassIntrospector { // Already overloaded method - add new overload ((OverloadedMethods) previous).addMethod(method); } else if (decision.getMethodShadowsProperty() - || !(previous instanceof PropertyDescriptor)) { + || !(previous instanceof FastPropertyDescriptor)) { // Simple method (this far) introspData.put(methodKey, method); Class<?>[] replaced = getArgTypesByMethod(introspData).put(method, @@ -661,45 +660,28 @@ class ClassIntrospector { private void addPropertyDescriptorToClassIntrospectionData(Map<Object, Object> introspData, PropertyDescriptor pd, Class<?> clazz, Map<MethodSignature, List<Method>> accessibleMethods) { + Method readMethod = getMatchingAccessibleMethod(pd.getReadMethod(), accessibleMethods); + if (readMethod != null && !isAllowedToExpose(readMethod)) { + readMethod = null; + } + + Method indexedReadMethod; if (pd instanceof IndexedPropertyDescriptor) { - IndexedPropertyDescriptor ipd = - (IndexedPropertyDescriptor) pd; - Method readMethod = ipd.getIndexedReadMethod(); - Method publicReadMethod = getMatchingAccessibleMethod(readMethod, accessibleMethods); - if (publicReadMethod != null && isAllowedToExpose(publicReadMethod)) { - try { - if (readMethod != publicReadMethod) { - ipd = new IndexedPropertyDescriptor( - ipd.getName(), ipd.getReadMethod(), - null, publicReadMethod, - null); - } - introspData.put(ipd.getName(), ipd); - getArgTypesByMethod(introspData).put(publicReadMethod, publicReadMethod.getParameterTypes()); - } catch (IntrospectionException e) { - LOG.warn("Failed creating a publicly-accessible " + - "property descriptor for " + clazz.getName() + - " indexed property " + pd.getName() + - ", read method " + publicReadMethod, - e); - } + indexedReadMethod = getMatchingAccessibleMethod( + ((IndexedPropertyDescriptor) pd).getIndexedReadMethod(), accessibleMethods); + if (indexedReadMethod != null && !isAllowedToExpose(indexedReadMethod)) { + indexedReadMethod = null; } - } else { - Method readMethod = pd.getReadMethod(); - Method publicReadMethod = getMatchingAccessibleMethod(readMethod, accessibleMethods); - if (publicReadMethod != null && isAllowedToExpose(publicReadMethod)) { - try { - if (readMethod != publicReadMethod) { - pd = new PropertyDescriptor(pd.getName(), publicReadMethod, null); - } - introspData.put(pd.getName(), pd); - } catch (IntrospectionException e) { - LOG.warn("Failed creating a publicly-accessible " + - "property descriptor for " + clazz.getName() + - " property " + pd.getName() + ", read method " + - publicReadMethod, e); - } + if (indexedReadMethod != null) { + getArgTypesByMethod(introspData).put( + indexedReadMethod, indexedReadMethod.getParameterTypes()); } + } else { + indexedReadMethod = null; + } + + if (readMethod != null || indexedReadMethod != null) { + introspData.put(pd.getName(), new FastPropertyDescriptor(readMethod, indexedReadMethod)); } } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/8f787963/src/main/java/freemarker/ext/beans/FastPropertyDescriptor.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/beans/FastPropertyDescriptor.java b/src/main/java/freemarker/ext/beans/FastPropertyDescriptor.java new file mode 100644 index 0000000..12d43de --- /dev/null +++ b/src/main/java/freemarker/ext/beans/FastPropertyDescriptor.java @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package freemarker.ext.beans; + +import java.beans.PropertyDescriptor; +import java.lang.reflect.Method; + +/** + * Used instead of {@link PropertyDescriptor}, because the methods of that are synchronized. + * + * @since 2.3.27 + */ +final class FastPropertyDescriptor { + private final Method readMethod; + private final Method indexedReadMethod; + + public FastPropertyDescriptor(Method readMethod, Method indexedReadMethod) { + this.readMethod = readMethod; + this.indexedReadMethod = indexedReadMethod; + } + + public Method getReadMethod() { + return readMethod; + } + + public Method getIndexedReadMethod() { + return indexedReadMethod; + } + +}
