Skip to content

Commit 0a877af

Browse files
committed
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
1 parent 228a775 commit 0a877af

File tree

5 files changed

+93
-67
lines changed

5 files changed

+93
-67
lines changed

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

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
public class HandlerMethod {
4747

4848
/** Logger that is available to subclasses */
49-
protected final Log logger = LogFactory.getLog(getClass());
49+
protected final Log logger = LogFactory.getLog(HandlerMethod.class);
5050

5151
private final Object bean;
5252

@@ -58,55 +58,61 @@ public class HandlerMethod {
5858

5959
private final Method bridgedMethod;
6060

61+
6162
/**
62-
* Constructs a new handler method with the given bean instance and method.
63-
* @param bean the object bean
64-
* @param method the method
63+
* Create an instance from a bean instance and a method.
6564
*/
6665
public HandlerMethod(Object bean, Method method) {
67-
Assert.notNull(bean, "bean must not be null");
68-
Assert.notNull(method, "method must not be null");
66+
Assert.notNull(bean, "bean is required");
67+
Assert.notNull(method, "method is required");
6968
this.bean = bean;
7069
this.beanFactory = null;
7170
this.method = method;
7271
this.bridgedMethod = BridgeMethodResolver.findBridgedMethod(method);
7372
}
7473

7574
/**
76-
* Constructs a new handler method with the given bean instance, method name and parameters.
77-
* @param bean the object bean
78-
* @param methodName the method name
79-
* @param parameterTypes the method parameter types
75+
* Create an instance from a bean instance, method name, and parameter types.
8076
* @throws NoSuchMethodException when the method cannot be found
8177
*/
8278
public HandlerMethod(Object bean, String methodName, Class<?>... parameterTypes) throws NoSuchMethodException {
83-
Assert.notNull(bean, "bean must not be null");
84-
Assert.notNull(methodName, "method must not be null");
79+
Assert.notNull(bean, "bean is required");
80+
Assert.notNull(methodName, "method is required");
8581
this.bean = bean;
8682
this.beanFactory = null;
8783
this.method = bean.getClass().getMethod(methodName, parameterTypes);
8884
this.bridgedMethod = BridgeMethodResolver.findBridgedMethod(method);
8985
}
9086

9187
/**
92-
* Constructs a new handler method with the given bean name and method. The bean name will be lazily
93-
* initialized when {@link #createWithResolvedBean()} is called.
94-
* @param beanName the bean name
95-
* @param beanFactory the bean factory to use for bean initialization
96-
* @param method the method for the bean
88+
* Create an instance from a bean name, a method, and a {@code BeanFactory}.
89+
* The method {@link #createWithResolvedBean()} may be used later to
90+
* re-create the {@code HandlerMethod} with an initialized the bean.
9791
*/
9892
public HandlerMethod(String beanName, BeanFactory beanFactory, Method method) {
99-
Assert.hasText(beanName, "'beanName' must not be null");
100-
Assert.notNull(beanFactory, "'beanFactory' must not be null");
101-
Assert.notNull(method, "'method' must not be null");
93+
Assert.hasText(beanName, "beanName is required");
94+
Assert.notNull(beanFactory, "beanFactory is required");
95+
Assert.notNull(method, "method is required");
10296
Assert.isTrue(beanFactory.containsBean(beanName),
103-
"Bean factory [" + beanFactory + "] does not contain bean " + "with name [" + beanName + "]");
97+
"Bean factory [" + beanFactory + "] does not contain bean [" + beanName + "]");
10498
this.bean = beanName;
10599
this.beanFactory = beanFactory;
106100
this.method = method;
107101
this.bridgedMethod = BridgeMethodResolver.findBridgedMethod(method);
108102
}
109103

104+
/**
105+
* Create an instance from another {@code HandlerMethod}.
106+
*/
107+
protected HandlerMethod(HandlerMethod handlerMethod) {
108+
Assert.notNull(handlerMethod, "HandlerMethod is required");
109+
this.bean = handlerMethod.bean;
110+
this.beanFactory = handlerMethod.beanFactory;
111+
this.method = handlerMethod.method;
112+
this.bridgedMethod = handlerMethod.bridgedMethod;
113+
this.parameters = handlerMethod.parameters;
114+
}
115+
110116
/**
111117
* Returns the bean for this handler method.
112118
*/
@@ -146,11 +152,10 @@ protected Method getBridgedMethod() {
146152
public MethodParameter[] getMethodParameters() {
147153
if (this.parameters == null) {
148154
int parameterCount = this.bridgedMethod.getParameterTypes().length;
149-
MethodParameter[] p = new MethodParameter[parameterCount];
155+
this.parameters = new MethodParameter[parameterCount];
150156
for (int i = 0; i < parameterCount; i++) {
151-
p[i] = new HandlerMethodParameter(i);
157+
this.parameters[i] = new HandlerMethodParameter(i);
152158
}
153-
this.parameters = p;
154159
}
155160
return this.parameters;
156161
}
@@ -196,7 +201,9 @@ public HandlerMethod createWithResolvedBean() {
196201
String beanName = (String) this.bean;
197202
handler = this.beanFactory.getBean(beanName);
198203
}
199-
return new HandlerMethod(handler, this.method);
204+
HandlerMethod handlerMethod = new HandlerMethod(handler, this.method);
205+
handlerMethod.parameters = getMethodParameters();
206+
return handlerMethod;
200207
}
201208

202209
@Override

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,21 @@ public class InvocableHandlerMethod extends HandlerMethod {
5353

5454
private ParameterNameDiscoverer parameterNameDiscoverer = new LocalVariableTableParameterNameDiscoverer();
5555

56+
5657
/**
57-
* Constructs a new handler method with the given bean instance and method.
58-
* @param bean the bean instance
59-
* @param method the method
58+
* Creates an instance from the given handler and method.
6059
*/
6160
public InvocableHandlerMethod(Object bean, Method method) {
6261
super(bean, method);
6362
}
6463

64+
/**
65+
* Create an instance from a {@code HandlerMethod}.
66+
*/
67+
public InvocableHandlerMethod(HandlerMethod handlerMethod) {
68+
super(handlerMethod);
69+
}
70+
6571
/**
6672
* Constructs a new handler method with the given bean instance, method name and parameters.
6773
* @param bean the object bean

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

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -36,19 +36,19 @@
3636

3737
/**
3838
* Test fixture for {@link InvocableHandlerMethod} unit tests.
39-
*
39+
*
4040
* @author Rossen Stoyanchev
4141
*/
4242
public class InvocableHandlerMethodTests {
4343

44-
private InvocableHandlerMethod handleMethod;
44+
private InvocableHandlerMethod handlerMethod;
4545

4646
private NativeWebRequest webRequest;
4747

4848
@Before
4949
public void setUp() throws Exception {
5050
Method method = Handler.class.getDeclaredMethod("handle", Integer.class, String.class);
51-
this.handleMethod = new InvocableHandlerMethod(new Handler(), method);
51+
this.handlerMethod = new InvocableHandlerMethod(new Handler(), method);
5252
this.webRequest = new ServletWebRequest(new MockHttpServletRequest(), new MockHttpServletResponse());
5353
}
5454

@@ -60,14 +60,14 @@ public void resolveArg() throws Exception {
6060
HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
6161
composite.addResolver(intResolver);
6262
composite.addResolver(stringResolver);
63-
handleMethod.setHandlerMethodArgumentResolvers(composite);
64-
65-
Object returnValue = handleMethod.invokeForRequest(webRequest, null);
66-
63+
handlerMethod.setHandlerMethodArgumentResolvers(composite);
64+
65+
Object returnValue = handlerMethod.invokeForRequest(webRequest, null);
66+
6767
assertEquals(1, intResolver.getResolvedParameters().size());
6868
assertEquals(1, stringResolver.getResolvedParameters().size());
6969
assertEquals("99-value", returnValue);
70-
70+
7171
assertEquals("intArg", intResolver.getResolvedParameters().get(0).getParameterName());
7272
assertEquals("stringArg", stringResolver.getResolvedParameters().get(0).getParameterName());
7373
}
@@ -80,10 +80,10 @@ public void resolveNullArg() throws Exception {
8080
HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
8181
composite.addResolver(intResolver);
8282
composite.addResolver(stringResolver);
83-
handleMethod.setHandlerMethodArgumentResolvers(composite);
84-
85-
Object returnValue = handleMethod.invokeForRequest(webRequest, null);
86-
83+
handlerMethod.setHandlerMethodArgumentResolvers(composite);
84+
85+
Object returnValue = handlerMethod.invokeForRequest(webRequest, null);
86+
8787
assertEquals(1, intResolver.getResolvedParameters().size());
8888
assertEquals(1, stringResolver.getResolvedParameters().size());
8989
assertEquals("null-null", returnValue);
@@ -92,7 +92,7 @@ public void resolveNullArg() throws Exception {
9292
@Test
9393
public void cannotResolveArg() throws Exception {
9494
try {
95-
handleMethod.invokeForRequest(webRequest, null);
95+
handlerMethod.invokeForRequest(webRequest, null);
9696
fail("Expected exception");
9797
} catch (IllegalStateException ex) {
9898
assertTrue(ex.getMessage().contains("No suitable resolver for argument [0] [type=java.lang.Integer]"));
@@ -101,7 +101,7 @@ public void cannotResolveArg() throws Exception {
101101

102102
@Test
103103
public void resolveProvidedArg() throws Exception {
104-
Object returnValue = handleMethod.invokeForRequest(webRequest, null, 99, "value");
104+
Object returnValue = handlerMethod.invokeForRequest(webRequest, null, 99, "value");
105105

106106
assertEquals(String.class, returnValue.getClass());
107107
assertEquals("99-value", returnValue);
@@ -115,21 +115,21 @@ public void resolveProvidedArgFirst() throws Exception {
115115
HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
116116
composite.addResolver(intResolver);
117117
composite.addResolver(stringResolver);
118-
handleMethod.setHandlerMethodArgumentResolvers(composite);
118+
handlerMethod.setHandlerMethodArgumentResolvers(composite);
119119

120-
Object returnValue = handleMethod.invokeForRequest(webRequest, null, 2, "value2");
120+
Object returnValue = handlerMethod.invokeForRequest(webRequest, null, 2, "value2");
121121

122122
assertEquals("2-value2", returnValue);
123123
}
124-
124+
125125
@Test
126126
public void exceptionInResolvingArg() throws Exception {
127127
HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
128128
composite.addResolver(new ExceptionRaisingArgumentResolver());
129-
handleMethod.setHandlerMethodArgumentResolvers(composite);
130-
129+
handlerMethod.setHandlerMethodArgumentResolvers(composite);
130+
131131
try {
132-
handleMethod.invokeForRequest(webRequest, null);
132+
handlerMethod.invokeForRequest(webRequest, null);
133133
fail("Expected exception");
134134
} catch (HttpMessageNotReadableException ex) {
135135
// Expected..
@@ -145,10 +145,10 @@ public void illegalArgumentException() throws Exception {
145145
HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
146146
composite.addResolver(intResolver);
147147
composite.addResolver(stringResolver);
148-
handleMethod.setHandlerMethodArgumentResolvers(composite);
148+
handlerMethod.setHandlerMethodArgumentResolvers(composite);
149149

150150
try {
151-
handleMethod.invokeForRequest(webRequest, null);
151+
handlerMethod.invokeForRequest(webRequest, null);
152152
fail("Expected exception");
153153
} catch (IllegalArgumentException ex) {
154154
assertNotNull("Exception not wrapped", ex.getCause());
@@ -200,30 +200,30 @@ private void invokeExceptionRaisingHandler(Throwable expected) throws Exception
200200
new InvocableHandlerMethod(handler, method).invokeForRequest(webRequest, null);
201201
fail("Expected exception");
202202
}
203-
203+
204204
@SuppressWarnings("unused")
205205
private static class Handler {
206-
206+
207207
public String handle(Integer intArg, String stringArg) {
208208
return intArg + "-" + stringArg;
209209
}
210210
}
211211

212212
@SuppressWarnings("unused")
213213
private static class ExceptionRaisingHandler {
214-
214+
215215
private final Throwable t;
216216

217217
public ExceptionRaisingHandler(Throwable t) {
218218
this.t = t;
219219
}
220-
220+
221221
public void raiseException() throws Throwable {
222222
throw t;
223223
}
224-
224+
225225
}
226-
226+
227227
private static class ExceptionRaisingArgumentResolver implements HandlerMethodArgumentResolver {
228228

229229
public boolean supportsParameter(MethodParameter parameter) {
@@ -235,5 +235,5 @@ public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer m
235235
throw new HttpMessageNotReadableException("oops, can't read");
236236
}
237237
}
238-
238+
239239
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ private ServletInvocableHandlerMethod createRequestMappingMethod(
727727
HandlerMethod handlerMethod, WebDataBinderFactory binderFactory) {
728728

729729
ServletInvocableHandlerMethod requestMethod;
730-
requestMethod = new ServletInvocableHandlerMethod(handlerMethod.getBean(), handlerMethod.getMethod());
730+
requestMethod = new ServletInvocableHandlerMethod(handlerMethod);
731731
requestMethod.setHandlerMethodArgumentResolvers(this.argumentResolvers);
732732
requestMethod.setHandlerMethodReturnValueHandlers(this.returnValueHandlers);
733733
requestMethod.setDataBinderFactory(binderFactory);

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

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.springframework.util.StringUtils;
2727
import org.springframework.web.bind.annotation.ResponseStatus;
2828
import org.springframework.web.context.request.ServletWebRequest;
29+
import org.springframework.web.method.HandlerMethod;
2930
import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
3031
import org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite;
3132
import org.springframework.web.method.support.InvocableHandlerMethod;
@@ -56,18 +57,28 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod {
5657

5758
private HandlerMethodReturnValueHandlerComposite returnValueHandlers;
5859

60+
5961
/**
60-
* Creates a {@link ServletInvocableHandlerMethod} instance with the given bean and method.
61-
* @param handler the object handler
62-
* @param method the method
62+
* Creates an instance from the given handler and method.
6363
*/
6464
public ServletInvocableHandlerMethod(Object handler, Method method) {
6565
super(handler, method);
66+
initResponseStatus();
67+
}
68+
69+
/**
70+
* Create an instance from a {@code HandlerMethod}.
71+
*/
72+
public ServletInvocableHandlerMethod(HandlerMethod handlerMethod) {
73+
super(handlerMethod);
74+
initResponseStatus();
75+
}
6676

67-
ResponseStatus annotation = getMethodAnnotation(ResponseStatus.class);
68-
if (annotation != null) {
69-
this.responseStatus = annotation.value();
70-
this.responseReason = annotation.reason();
77+
private void initResponseStatus() {
78+
ResponseStatus annot = getMethodAnnotation(ResponseStatus.class);
79+
if (annot != null) {
80+
this.responseStatus = annot.value();
81+
this.responseReason = annot.reason();
7182
}
7283
}
7384

@@ -185,7 +196,9 @@ else if (result instanceof Throwable) {
185196

186197

187198
/**
188-
* Wrap a Callable as a ServletInvocableHandlerMethod inheriting method-level annotations.
199+
* A ServletInvocableHandlerMethod sub-class that invokes a given
200+
* {@link Callable} and "inherits" the annotations of the containing class
201+
* instance, useful for invoking a Callable returned from a HandlerMethod.
189202
*/
190203
private class CallableHandlerMethod extends ServletInvocableHandlerMethod {
191204

0 commit comments

Comments
 (0)