Optimize use of HandlerMethod and sub-classes

While HandlerMethod instances are cached for lookup purposes, a new
ServletInvocableHandlerMethod instance has to be created prior to each
invocation since handlers may have non-singleton scope semantics.

This change reduces the overhead of creating per request instances
by using a logger with a fixed name rather than relying on getClass()
and also by copying introspected method parameters from the cached
HandlerMethod instance.

Issue: SPR-9747, SPR-9748

spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java

@@ -46,7 +46,7 @@ import org.springframework.util.ClassUtils;
 public class HandlerMethod {
 
 	/** Logger that is available to subclasses */
-	protected final Log logger = LogFactory.getLog(getClass());
+	protected final Log logger = LogFactory.getLog(HandlerMethod.class);
 
 	private final Object bean;
 
@@ -58,14 +58,13 @@ public class HandlerMethod {
 
 	private final Method bridgedMethod;
 
+
 	/**
-	 * Constructs a new handler method with the given bean instance and method.
+	 * Create an instance from a bean instance and a method.
-	 * @param bean the object bean
-	 * @param method the method
 	 */
 	public HandlerMethod(Object bean, Method method) {
-		Assert.notNull(bean, "bean must not be null");
+		Assert.notNull(bean, "bean is required");
-		Assert.notNull(method, "method must not be null");
+		Assert.notNull(method, "method is required");
 		this.bean = bean;
 		this.beanFactory = null;
 		this.method = method;
@@ -73,15 +72,12 @@ public class HandlerMethod {
 	}
 
 	/**
-	 * Constructs a new handler method with the given bean instance, method name and parameters.
+	 * Create an instance from a bean instance, method name, and parameter types.
-	 * @param bean the object bean
-	 * @param methodName the method name
-	 * @param parameterTypes the method parameter types
 	 * @throws NoSuchMethodException when the method cannot be found
 	 */
 	public HandlerMethod(Object bean, String methodName, Class<?>... parameterTypes) throws NoSuchMethodException {
-		Assert.notNull(bean, "bean must not be null");
+		Assert.notNull(bean, "bean is required");
-		Assert.notNull(methodName, "method must not be null");
+		Assert.notNull(methodName, "method is required");
 		this.bean = bean;
 		this.beanFactory = null;
 		this.method = bean.getClass().getMethod(methodName, parameterTypes);
@@ -89,24 +85,34 @@ public class HandlerMethod {
 	}
 
 	/**
-	 * Constructs a new handler method with the given bean name and method. The bean name will be lazily
+	 * Create an instance from a bean name, a method, and a {@code BeanFactory}.
-	 * initialized when {@link #createWithResolvedBean()} is called.
+	 * The method {@link #createWithResolvedBean()} may be used later to
-	 * @param beanName the bean name
+	 * re-create the {@code HandlerMethod} with an initialized the bean.
-	 * @param beanFactory the bean factory to use for bean initialization
-	 * @param method the method for the bean
 	 */
 	public HandlerMethod(String beanName, BeanFactory beanFactory, Method method) {
-		Assert.hasText(beanName, "'beanName' must not be null");
+		Assert.hasText(beanName, "beanName is required");
-		Assert.notNull(beanFactory, "'beanFactory' must not be null");
+		Assert.notNull(beanFactory, "beanFactory is required");
-		Assert.notNull(method, "'method' must not be null");
+		Assert.notNull(method, "method is required");
 		Assert.isTrue(beanFactory.containsBean(beanName),
-				"Bean factory [" + beanFactory + "] does not contain bean " + "with name [" + beanName + "]");
+				"Bean factory [" + beanFactory + "] does not contain bean [" + beanName + "]");
 		this.bean = beanName;
 		this.beanFactory = beanFactory;
 		this.method = method;
 		this.bridgedMethod = BridgeMethodResolver.findBridgedMethod(method);
 	}
 
+	/**
+	 * Create an instance from another {@code HandlerMethod}.
+	 */
+	protected HandlerMethod(HandlerMethod handlerMethod) {
+		Assert.notNull(handlerMethod, "HandlerMethod is required");
+		this.bean = handlerMethod.bean;
+		this.beanFactory = handlerMethod.beanFactory;
+		this.method = handlerMethod.method;
+		this.bridgedMethod = handlerMethod.bridgedMethod;
+		this.parameters = handlerMethod.parameters;
+	}
+
 	/**
 	 * Returns the bean for this handler method.
 	 */
@@ -146,11 +152,10 @@ public class HandlerMethod {
 	public MethodParameter[] getMethodParameters() {
 		if (this.parameters == null) {
 			int parameterCount = this.bridgedMethod.getParameterTypes().length;
-			MethodParameter[] p = new MethodParameter[parameterCount];
+			this.parameters = new MethodParameter[parameterCount];
 			for (int i = 0; i < parameterCount; i++) {
-				p[i] = new HandlerMethodParameter(i);
+				this.parameters[i] = new HandlerMethodParameter(i);
 			}
-			this.parameters = p;
 		}
 		return this.parameters;
 	}
@@ -196,7 +201,9 @@ public class HandlerMethod {
 			String beanName = (String) this.bean;
 			handler = this.beanFactory.getBean(beanName);
 		}
-		return new HandlerMethod(handler, this.method);
+		HandlerMethod handlerMethod = new HandlerMethod(handler, this.method);
+		handlerMethod.parameters = getMethodParameters();
+		return handlerMethod;
 	}
 
 	@Override

spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java

@@ -53,15 +53,21 @@ public class InvocableHandlerMethod extends HandlerMethod {
 
 	private ParameterNameDiscoverer parameterNameDiscoverer = new LocalVariableTableParameterNameDiscoverer();
 
+
 	/**
-	 * Constructs a new handler method with the given bean instance and method.
+	 * Creates an instance from the given handler and method.
-	 * @param bean the bean instance
-	 * @param method the method
 	 */
 	public InvocableHandlerMethod(Object bean, Method method) {
 		super(bean, method);
 	}
 
+	/**
+	 * Create an instance from a {@code HandlerMethod}.
+	 */
+	public InvocableHandlerMethod(HandlerMethod handlerMethod) {
+		super(handlerMethod);
+	}
+
 	/**
 	 * Constructs a new handler method with the given bean instance, method name and parameters.
 	 * @param bean the object bean

spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -36,19 +36,19 @@ import org.springframework.web.context.request.ServletWebRequest;
 
 /**
  * Test fixture for {@link InvocableHandlerMethod} unit tests.
- * 
+ *
  * @author Rossen Stoyanchev
  */
 public class InvocableHandlerMethodTests {
 
-	private InvocableHandlerMethod handleMethod;
+	private InvocableHandlerMethod handlerMethod;
 
 	private NativeWebRequest webRequest;
 
 	@Before
 	public void setUp() throws Exception {
 		Method method = Handler.class.getDeclaredMethod("handle", Integer.class, String.class);
-		this.handleMethod = new InvocableHandlerMethod(new Handler(), method);
+		this.handlerMethod = new InvocableHandlerMethod(new Handler(), method);
 		this.webRequest = new ServletWebRequest(new MockHttpServletRequest(), new MockHttpServletResponse());
 	}
 
@@ -60,14 +60,14 @@ public class InvocableHandlerMethodTests {
 		HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 		composite.addResolver(intResolver);
 		composite.addResolver(stringResolver);
-		handleMethod.setHandlerMethodArgumentResolvers(composite);
+		handlerMethod.setHandlerMethodArgumentResolvers(composite);
-		
+
-		Object returnValue = handleMethod.invokeForRequest(webRequest, null);
+		Object returnValue = handlerMethod.invokeForRequest(webRequest, null);
-		
+
 		assertEquals(1, intResolver.getResolvedParameters().size());
 		assertEquals(1, stringResolver.getResolvedParameters().size());
 		assertEquals("99-value", returnValue);
-		
+
 		assertEquals("intArg", intResolver.getResolvedParameters().get(0).getParameterName());
 		assertEquals("stringArg", stringResolver.getResolvedParameters().get(0).getParameterName());
 	}
@@ -80,10 +80,10 @@ public class InvocableHandlerMethodTests {
 		HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 		composite.addResolver(intResolver);
 		composite.addResolver(stringResolver);
-		handleMethod.setHandlerMethodArgumentResolvers(composite);
+		handlerMethod.setHandlerMethodArgumentResolvers(composite);
-		
+
-		Object returnValue = handleMethod.invokeForRequest(webRequest, null);
+		Object returnValue = handlerMethod.invokeForRequest(webRequest, null);
-		
+
 		assertEquals(1, intResolver.getResolvedParameters().size());
 		assertEquals(1, stringResolver.getResolvedParameters().size());
 		assertEquals("null-null", returnValue);
@@ -92,7 +92,7 @@ public class InvocableHandlerMethodTests {
 	@Test
 	public void cannotResolveArg() throws Exception {
 		try {
-			handleMethod.invokeForRequest(webRequest, null);
+			handlerMethod.invokeForRequest(webRequest, null);
 			fail("Expected exception");
 		} catch (IllegalStateException ex) {
 			assertTrue(ex.getMessage().contains("No suitable resolver for argument [0] [type=java.lang.Integer]"));
@@ -101,7 +101,7 @@ public class InvocableHandlerMethodTests {
 
 	@Test
 	public void resolveProvidedArg() throws Exception {
-		Object returnValue = handleMethod.invokeForRequest(webRequest, null, 99, "value");
+		Object returnValue = handlerMethod.invokeForRequest(webRequest, null, 99, "value");
 
 		assertEquals(String.class, returnValue.getClass());
 		assertEquals("99-value", returnValue);
@@ -115,21 +115,21 @@ public class InvocableHandlerMethodTests {
 		HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 		composite.addResolver(intResolver);
 		composite.addResolver(stringResolver);
-		handleMethod.setHandlerMethodArgumentResolvers(composite);
+		handlerMethod.setHandlerMethodArgumentResolvers(composite);
 
-		Object returnValue = handleMethod.invokeForRequest(webRequest, null, 2, "value2");
+		Object returnValue = handlerMethod.invokeForRequest(webRequest, null, 2, "value2");
 
 		assertEquals("2-value2", returnValue);
 	}
-	
+
 	@Test
 	public void exceptionInResolvingArg() throws Exception {
 		HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 		composite.addResolver(new ExceptionRaisingArgumentResolver());
-		handleMethod.setHandlerMethodArgumentResolvers(composite);
+		handlerMethod.setHandlerMethodArgumentResolvers(composite);
-		
+
 		try {
-			handleMethod.invokeForRequest(webRequest, null);
+			handlerMethod.invokeForRequest(webRequest, null);
 			fail("Expected exception");
 		} catch (HttpMessageNotReadableException ex) {
 			// Expected..
@@ -145,10 +145,10 @@ public class InvocableHandlerMethodTests {
 		HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 		composite.addResolver(intResolver);
 		composite.addResolver(stringResolver);
-		handleMethod.setHandlerMethodArgumentResolvers(composite);
+		handlerMethod.setHandlerMethodArgumentResolvers(composite);
 
 		try {
-			handleMethod.invokeForRequest(webRequest, null);
+			handlerMethod.invokeForRequest(webRequest, null);
 			fail("Expected exception");
 		} catch (IllegalArgumentException ex) {
 			assertNotNull("Exception not wrapped", ex.getCause());
@@ -200,10 +200,10 @@ public class InvocableHandlerMethodTests {
 		new InvocableHandlerMethod(handler, method).invokeForRequest(webRequest, null);
 		fail("Expected exception");
 	}
-	
+
 	@SuppressWarnings("unused")
 	private static class Handler {
-		
+
 		public String handle(Integer intArg, String stringArg) {
 			return intArg + "-" + stringArg;
 		}
@@ -211,19 +211,19 @@ public class InvocableHandlerMethodTests {
 
 	@SuppressWarnings("unused")
 	private static class ExceptionRaisingHandler {
-		
+
 		private final Throwable t;
 
 		public ExceptionRaisingHandler(Throwable t) {
 			this.t = t;
 		}
-		
+
 		public void raiseException() throws Throwable {
 			throw t;
 		}
-		
+
 	}
-	
+
 	private static class ExceptionRaisingArgumentResolver implements HandlerMethodArgumentResolver {
 
 		public boolean supportsParameter(MethodParameter parameter) {
@@ -235,5 +235,5 @@ public class InvocableHandlerMethodTests {
 			throw new HttpMessageNotReadableException("oops, can't read");
 		}
 	}
-	
+
 }

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java

@@ -727,7 +727,7 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter i
 			HandlerMethod handlerMethod, WebDataBinderFactory binderFactory) {
 
 		ServletInvocableHandlerMethod requestMethod;
-		requestMethod = new ServletInvocableHandlerMethod(handlerMethod.getBean(), handlerMethod.getMethod());
+		requestMethod = new ServletInvocableHandlerMethod(handlerMethod);
 		requestMethod.setHandlerMethodArgumentResolvers(this.argumentResolvers);
 		requestMethod.setHandlerMethodReturnValueHandlers(this.returnValueHandlers);
 		requestMethod.setDataBinderFactory(binderFactory);

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java

@@ -26,6 +26,7 @@ import org.springframework.util.ClassUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.context.request.ServletWebRequest;
+import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
 import org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite;
 import org.springframework.web.method.support.InvocableHandlerMethod;
@@ -56,18 +57,28 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod {
 
 	private HandlerMethodReturnValueHandlerComposite returnValueHandlers;
 
+
 	/**
-	 * Creates a {@link ServletInvocableHandlerMethod} instance with the given bean and method.
+	 * Creates an instance from the given handler and method.
-	 * @param handler the object handler
-	 * @param method the method
 	 */
 	public ServletInvocableHandlerMethod(Object handler, Method method) {
 		super(handler, method);
+		initResponseStatus();
+	}
+
+	/**
+	 * Create an instance from a {@code HandlerMethod}.
+	 */
+	public ServletInvocableHandlerMethod(HandlerMethod handlerMethod) {
+		super(handlerMethod);
+		initResponseStatus();
+	}
 
-		ResponseStatus annotation = getMethodAnnotation(ResponseStatus.class);
+	private void initResponseStatus() {
-		if (annotation != null) {
+		ResponseStatus annot = getMethodAnnotation(ResponseStatus.class);
-			this.responseStatus = annotation.value();
+		if (annot != null) {
-			this.responseReason = annotation.reason();
+			this.responseStatus = annot.value();
+			this.responseReason = annot.reason();
 		}
 	}
 
@@ -185,7 +196,9 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod {
 
 
 	/**
-	 * Wrap a Callable as a ServletInvocableHandlerMethod inheriting method-level annotations.
+	 * A ServletInvocableHandlerMethod sub-class that invokes a given
+	 * {@link Callable} and "inherits" the annotations of the containing class
+	 * instance, useful for invoking a Callable returned from a HandlerMethod.
 	 */
 	private class CallableHandlerMethod extends ServletInvocableHandlerMethod {