From 4838e9f40c8a1082a3af1fb2d4013493b318a550 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 21 Nov 2016 12:53:23 +0100 Subject: [PATCH] #511 - Performance improvements in link building. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We're now caching meta information about controller methods and their parameters more aggressively to avoid repeated lookups of such information via reflection. We're also keeping reference to proxy classes generated through CGlib around to avoid costly regeneration. Note that the main culprit causing the drastic numbers reported in the original ticket stem from a problem in Spring's TypeDescriptor which triggers costly equals(…) comparisons on synthesized annotations [0]. Removed conditional reflective lookup of DefaultParameterNameDiscoverer as it's now present by definition. [0] https://jira.spring.io/browse/SPR-14926 --- .../hateoas/core/DummyInvocationUtils.java | 52 ++++++++++++++++--- .../hateoas/core/MethodParameters.java | 32 +++++------- .../AnnotatedParametersParameterAccessor.java | 42 ++++++++++++--- 3 files changed, 95 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/core/DummyInvocationUtils.java b/src/main/java/org/springframework/hateoas/core/DummyInvocationUtils.java index 9f4b5553..db470a43 100644 --- a/src/main/java/org/springframework/hateoas/core/DummyInvocationUtils.java +++ b/src/main/java/org/springframework/hateoas/core/DummyInvocationUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ package org.springframework.hateoas.core; import java.lang.reflect.Method; import java.util.Arrays; import java.util.Iterator; +import java.util.Map; import org.aopalliance.intercept.MethodInterceptor; import org.springframework.aop.framework.ProxyFactory; @@ -28,6 +29,8 @@ import org.springframework.cglib.proxy.Factory; import org.springframework.cglib.proxy.MethodProxy; import org.springframework.objenesis.ObjenesisStd; import org.springframework.util.Assert; +import org.springframework.util.ConcurrentReferenceHashMap; +import org.springframework.util.ConcurrentReferenceHashMap.ReferenceType; import org.springframework.util.ReflectionUtils; /** @@ -37,7 +40,9 @@ import org.springframework.util.ReflectionUtils; */ public class DummyInvocationUtils { - private static ObjenesisStd OBJENESIS = new ObjenesisStd(); + private static final ObjenesisStd OBJENESIS = new ObjenesisStd(); + private static final Map, Class> CLASS_CACHE = new ConcurrentReferenceHashMap, Class>(16, + ReferenceType.WEAK); public interface LastInvocationAware { @@ -52,8 +57,8 @@ public class DummyInvocationUtils { * * @author Oliver Gierke */ - private static class InvocationRecordingMethodInterceptor implements MethodInterceptor, LastInvocationAware, - org.springframework.cglib.proxy.MethodInterceptor { + private static class InvocationRecordingMethodInterceptor + implements MethodInterceptor, LastInvocationAware, org.springframework.cglib.proxy.MethodInterceptor { private static final Method GET_INVOCATIONS; private static final Method GET_OBJECT_PARAMETERS; @@ -71,10 +76,14 @@ public class DummyInvocationUtils { * Creates a new {@link InvocationRecordingMethodInterceptor} carrying the given parameters forward that might be * needed to populate the class level mapping. * - * @param parameters + * @param targetType must not be {@literal null}. + * @param parameters must not be {@literal null}. */ public InvocationRecordingMethodInterceptor(Class targetType, Object... parameters) { + Assert.notNull(targetType, "Target type must not be null!"); + Assert.notNull(parameters, "Parameters must not be null!"); + this.targetType = targetType; this.objectParameters = parameters.clone(); } @@ -168,7 +177,7 @@ public class DummyInvocationUtils { enhancer.setCallbackType(org.springframework.cglib.proxy.MethodInterceptor.class); enhancer.setClassLoader(classLoader); - Factory factory = (Factory) OBJENESIS.newInstance(enhancer.createClass()); + Factory factory = (Factory) OBJENESIS.newInstance(getOrCreateEnhancedClass(type, classLoader)); factory.setCallbacks(new Callback[] { interceptor }); return (T) factory; } @@ -182,6 +191,37 @@ public class DummyInvocationUtils { Class getTargetType(); } + /** + * Returns the already created proxy class for the given source type or creates a new one. + * + * @param type must not be {@literal null}. + * @param classLoader must not be {@literal null}. + * @return + */ + private static Class getOrCreateEnhancedClass(Class type, ClassLoader classLoader) { + + Assert.notNull(type, "Source type must not be null!"); + Assert.notNull(classLoader, "ClassLoader must not be null!"); + + Class result = CLASS_CACHE.get(type); + + if (result != null) { + return result; + } + + Enhancer enhancer = new Enhancer(); + enhancer.setSuperclass(type); + enhancer.setInterfaces(new Class[] { LastInvocationAware.class }); + enhancer.setCallbackType(org.springframework.cglib.proxy.MethodInterceptor.class); + enhancer.setClassLoader(classLoader); + + result = enhancer.createClass(); + + CLASS_CACHE.put(type, result); + + return result; + } + static class SimpleMethodInvocation implements MethodInvocation { private final Class targetType; diff --git a/src/main/java/org/springframework/hateoas/core/MethodParameters.java b/src/main/java/org/springframework/hateoas/core/MethodParameters.java index 87f92784..2bd55ad8 100644 --- a/src/main/java/org/springframework/hateoas/core/MethodParameters.java +++ b/src/main/java/org/springframework/hateoas/core/MethodParameters.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2014 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,13 +19,13 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; +import java.util.Map; -import org.springframework.beans.BeanUtils; -import org.springframework.core.LocalVariableTableParameterNameDiscoverer; +import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.MethodParameter; import org.springframework.core.ParameterNameDiscoverer; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; +import org.springframework.util.ConcurrentReferenceHashMap; /** * Value object to represent {@link MethodParameters} to allow to easily find the ones with a given annotation. @@ -34,22 +34,10 @@ import org.springframework.util.ClassUtils; */ public class MethodParameters { - private static final String SPRING_4_DISCOVERER_NAME = "org.springframework.core.DefaultParameterNameDiscoverer"; - private static ParameterNameDiscoverer DISCOVERER; - - static { - - ClassLoader classLoader = MethodParameters.class.getClassLoader(); - - try { - Class discovererType = ClassUtils.forName(SPRING_4_DISCOVERER_NAME, classLoader); - DISCOVERER = (ParameterNameDiscoverer) BeanUtils.instantiate(discovererType); - } catch (ClassNotFoundException o_O) { - DISCOVERER = new LocalVariableTableParameterNameDiscoverer(); - } - } + private static ParameterNameDiscoverer DISCOVERER = new DefaultParameterNameDiscoverer(); private final List parameters; + private final Map, List> parametersWithAnnotationCache = new ConcurrentReferenceHashMap, List>(); /** * Creates a new {@link MethodParameters} from the given {@link Method}. @@ -137,6 +125,12 @@ public class MethodParameters { */ public List getParametersWith(Class annotation) { + List cached = parametersWithAnnotationCache.get(annotation); + + if (cached != null) { + return cached; + } + Assert.notNull(annotation); List result = new ArrayList(); @@ -146,6 +140,8 @@ public class MethodParameters { } } + parametersWithAnnotationCache.put(annotation, result); + return result; } diff --git a/src/main/java/org/springframework/hateoas/mvc/AnnotatedParametersParameterAccessor.java b/src/main/java/org/springframework/hateoas/mvc/AnnotatedParametersParameterAccessor.java index 06b744db..5284870d 100644 --- a/src/main/java/org/springframework/hateoas/mvc/AnnotatedParametersParameterAccessor.java +++ b/src/main/java/org/springframework/hateoas/mvc/AnnotatedParametersParameterAccessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2013 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,8 +16,10 @@ package org.springframework.hateoas.mvc; import java.lang.annotation.Annotation; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; +import java.util.Map; import org.springframework.core.MethodParameter; import org.springframework.core.convert.ConversionService; @@ -27,6 +29,8 @@ import org.springframework.hateoas.core.AnnotationAttribute; import org.springframework.hateoas.core.DummyInvocationUtils.MethodInvocation; import org.springframework.hateoas.core.MethodParameters; import org.springframework.util.Assert; +import org.springframework.util.ConcurrentReferenceHashMap; +import org.springframework.util.ConcurrentReferenceHashMap.ReferenceType; import org.springframework.util.StringUtils; import org.springframework.web.util.UriTemplate; @@ -37,6 +41,9 @@ import org.springframework.web.util.UriTemplate; */ class AnnotatedParametersParameterAccessor { + private static final Map METHOD_PARAMETERS_CACHE = new ConcurrentReferenceHashMap( + 16, ReferenceType.WEAK); + private final AnnotationAttribute attribute; /** @@ -47,6 +54,7 @@ class AnnotatedParametersParameterAccessor { public AnnotatedParametersParameterAccessor(AnnotationAttribute attribute) { Assert.notNull(attribute); + this.attribute = attribute; } @@ -60,7 +68,7 @@ class AnnotatedParametersParameterAccessor { Assert.notNull(invocation, "MethodInvocation must not be null!"); - MethodParameters parameters = new MethodParameters(invocation.getMethod()); + MethodParameters parameters = getOrCreateMethodParametersFor(invocation.getMethod()); Object[] arguments = invocation.getArguments(); List result = new ArrayList(); @@ -89,17 +97,37 @@ class AnnotatedParametersParameterAccessor { if (value == null) { - Object indexOrName = StringUtils.hasText(parameter.getParameterName()) ? parameter.getParameterName() : parameter - .getParameterIndex(); + Object indexOrName = StringUtils.hasText(parameter.getParameterName()) ? parameter.getParameterName() + : parameter.getParameterIndex(); - throw new IllegalArgumentException(String.format( - "Required controller parameter %s of method %s found but null value given!", indexOrName, - parameter.getMethod())); + throw new IllegalArgumentException( + String.format("Required controller parameter %s of method %s found but null value given!", indexOrName, + parameter.getMethod())); } return value; } + /** + * Returns the {@link MethodParameters} for the given {@link Method}. + * + * @param method + * @return + */ + private static MethodParameters getOrCreateMethodParametersFor(Method method) { + + MethodParameters methodParameters = METHOD_PARAMETERS_CACHE.get(method); + + if (methodParameters != null) { + return methodParameters; + } + + methodParameters = new MethodParameters(method); + METHOD_PARAMETERS_CACHE.put(method, methodParameters); + + return methodParameters; + } + /** * Represents a {@link MethodParameter} alongside the value it has been bound to. *