Skip to content

Commit 313c9d3

Browse files
odrotbohmodrotbohm
committed
#1701 - Fix performance degradations introduced by #467.
The commits for #467 significantly degraded performance as CachingMappingDiscoverer.getParams(Method) doesn't properly cache the result of the call which causes quite expensive, unnecessarily repeated annotation lookups for the very same method. We also avoid the creation of an Optional instance for the sole purpose of a simple null check. Introduce FormatterFactory to potentially cache the to-String formatting functions and thus avoid repeated evaluation and Function object creation. We now also avoid the creation of ParamRequestCondition instances if no @RequestParams(params = …) values could be found in the first place.
1 parent 4182698 commit 313c9d3

File tree

5 files changed

+100
-54
lines changed

5 files changed

+100
-54
lines changed

‎src/main/java/org/springframework/hateoas/server/core/AnnotationMappingDiscoverer.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package org.springframework.hateoas.server.core;
1717

18-
import static java.util.Optional.*;
1918
import static org.springframework.core.annotation.AnnotatedElementUtils.*;
2019
import static org.springframework.core.annotation.AnnotationUtils.*;
2120

@@ -177,7 +176,7 @@ public String[] getParams(Method method) {
177176
Annotation annotation = findMergedAnnotation(method, annotationType);
178177
String[] params = (String[]) getValue(annotation, "params");
179178

180-
return ofNullable(params).orElseGet(() -> new String[0]);
179+
return params == null ? new String[0] : params;
181180
}
182181

183182
private String[] getMappingFrom(@Nullable Annotation annotation) {

‎src/main/java/org/springframework/hateoas/server/core/CachingMappingDiscoverer.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ public class CachingMappingDiscoverer implements MappingDiscoverer {
3737

3838
private static final Map<String, String> MAPPINGS = new ConcurrentReferenceHashMap<>();
3939
private static final Map<String, Collection<HttpMethod>> METHODS = new ConcurrentReferenceHashMap<>();
40+
private static final Map<String, String[]> PARAMS = new ConcurrentReferenceHashMap<>();
41+
private static final Map<String, List<MediaType>> CONSUMES = new ConcurrentReferenceHashMap<>();
4042

4143
private final MappingDiscoverer delegate;
4244

@@ -68,10 +70,7 @@ public String getMapping(Class<?> type) {
6870
@Nullable
6971
@Override
7072
public String getMapping(Method method) {
71-
72-
String key = key(method.getDeclaringClass(), method);
73-
74-
return MAPPINGS.computeIfAbsent(key, __ -> delegate.getMapping(method));
73+
return MAPPINGS.computeIfAbsent(key(method), __ -> delegate.getMapping(method));
7574
}
7675

7776
/*
@@ -102,7 +101,7 @@ public Collection<HttpMethod> getRequestMethod(Class<?> type, Method method) {
102101
*/
103102
@Override
104103
public List<MediaType> getConsumes(Method method) {
105-
return delegate.getConsumes(method);
104+
return CONSUMES.computeIfAbsent(key(method), __ -> delegate.getConsumes(method));
106105
}
107106

108107
/*
@@ -111,7 +110,11 @@ public List<MediaType> getConsumes(Method method) {
111110
*/
112111
@Override
113112
public String[] getParams(Method method) {
114-
return delegate.getParams(method);
113+
return PARAMS.computeIfAbsent(key(method), __ -> delegate.getParams(method));
114+
}
115+
116+
private static String key(Method method) {
117+
return key(method.getDeclaringClass(), method);
115118
}
116119

117120
private static String key(Class<?> type, @Nullable Method method) {

‎src/main/java/org/springframework/hateoas/server/core/SpringAffordanceBuilder.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
*/
4646
public class SpringAffordanceBuilder {
4747

48-
@SuppressWarnings("deprecation") //
4948
public static final MappingDiscoverer DISCOVERER = CachingMappingDiscoverer
5049
.of(new PropertyResolvingMappingDiscoverer(new AnnotationMappingDiscoverer(RequestMapping.class)));
5150

‎src/main/java/org/springframework/hateoas/server/core/WebHandler.java

Lines changed: 76 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ public static <T extends LinkBuilder> T linkTo(Object invocationValue, LinkBuild
7878
@Nullable BiFunction<UriComponentsBuilder, MethodInvocation, UriComponentsBuilder> additionalUriHandler,
7979
Function<String, UriComponentsBuilder> finisher, Supplier<ConversionService> conversionService) {
8080

81-
return linkTo(invocationValue, creator, additionalUriHandler).conclude(finisher, conversionService.get());
81+
return linkTo(invocationValue, creator, additionalUriHandler).conclude(finisher,
82+
conversionService.get());
8283
}
8384

8485
private static <T extends LinkBuilder> PreparedWebHandler<T> linkTo(Object invocationValue,
@@ -99,6 +100,8 @@ private static <T extends LinkBuilder> PreparedWebHandler<T> linkTo(Object invoc
99100

100101
return (finisher, conversionService) -> {
101102

103+
FormatterFactory factory = new FormatterFactory(conversionService);
104+
102105
UriComponentsBuilder builder = finisher.apply(mapping);
103106
UriTemplate template = UriTemplateFactory.templateFor(mapping == null ? "/" : mapping);
104107
Map<String, Object> values = new HashMap<>();
@@ -114,7 +117,7 @@ private static <T extends LinkBuilder> PreparedWebHandler<T> linkTo(Object invoc
114117
Object source = classMappingParameters.next();
115118

116119
values.put(name, variable.prepareAndEncode(
117-
HandlerMethodParameter.prepareValue(source, conversionService, TypeDescriptor.forObject(source))));
120+
HandlerMethodParameter.prepareValue(source, factory, TypeDescriptor.forObject(source))));
118121
}
119122

120123
Method method = invocation.getMethod();
@@ -126,7 +129,7 @@ private static <T extends LinkBuilder> PreparedWebHandler<T> linkTo(Object invoc
126129
TemplateVariable variable = TemplateVariable.segment(parameter.getVariableName());
127130
Object verifiedValue = parameter.getVerifiedValue(arguments);
128131
Object preparedValue = verifiedValue == null ? verifiedValue
129-
: parameter.prepareValue(verifiedValue, conversionService);
132+
: parameter.prepareValue(verifiedValue, factory);
130133

131134
values.put(variable.getName(), variable.prepareAndEncode(preparedValue));
132135
}
@@ -135,7 +138,7 @@ private static <T extends LinkBuilder> PreparedWebHandler<T> linkTo(Object invoc
135138

136139
for (HandlerMethodParameter parameter : parameters.getParameterAnnotatedWith(RequestParam.class, arguments)) {
137140

138-
bindRequestParameters(builder, parameter, arguments, conversionService);
141+
bindRequestParameters(builder, parameter, arguments, factory);
139142

140143
boolean isSkipValue = SKIP_VALUE.equals(parameter.getVerifiedValue(arguments));
141144
boolean isMapParameter = Map.class.isAssignableFrom(parameter.parameter.getParameterType());
@@ -186,7 +189,7 @@ private static <T extends LinkBuilder> PreparedWebHandler<T> linkTo(Object invoc
186189
*/
187190
@SuppressWarnings("unchecked")
188191
private static void bindRequestParameters(UriComponentsBuilder builder, HandlerMethodParameter parameter,
189-
Object[] arguments, ConversionService conversionService) {
192+
Object[] arguments, FormatterFactory factory) {
190193

191194
Object value = parameter.getVerifiedValue(arguments);
192195

@@ -198,7 +201,7 @@ private static void bindRequestParameters(UriComponentsBuilder builder, HandlerM
198201

199202
if (value instanceof MultiValueMap) {
200203

201-
Map<String, List<?>> requestParams = (Map<String, List<?>>) parameter.prepareValue(value, conversionService);
204+
Map<String, List<?>> requestParams = (Map<String, List<?>>) parameter.prepareValue(value, factory);
202205

203206
for (Entry<String, List<?>> entry : requestParams.entrySet()) {
204207
for (Object element : entry.getValue()) {
@@ -212,7 +215,7 @@ private static void bindRequestParameters(UriComponentsBuilder builder, HandlerM
212215

213216
if (value instanceof Map) {
214217

215-
Map<String, ?> requestParams = (Map<String, ?>) parameter.prepareValue(value, conversionService);
218+
Map<String, ?> requestParams = (Map<String, ?>) parameter.prepareValue(value, factory);
216219

217220
for (Entry<String, ?> entry : requestParams.entrySet()) {
218221

@@ -234,7 +237,7 @@ private static void bindRequestParameters(UriComponentsBuilder builder, HandlerM
234237

235238
if (value instanceof Collection) {
236239

237-
Collection<?> collection = (Collection<?>) parameter.prepareValue(value, conversionService);
240+
Collection<?> collection = (Collection<?>) parameter.prepareValue(value, factory);
238241

239242
if (parameter.isNonComposite()) {
240243
builder.queryParam(key, variable.prepareAndEncode(collection));
@@ -256,31 +259,11 @@ private static void bindRequestParameters(UriComponentsBuilder builder, HandlerM
256259

257260
} else {
258261
if (key != null) {
259-
builder.queryParam(key, variable.prepareAndEncode(parameter.prepareValue(value, conversionService)));
262+
builder.queryParam(key, variable.prepareAndEncode(parameter.prepareValue(value, factory)));
260263
}
261264
}
262265
}
263266

264-
private static Function<Object, String> getFormatter(ConversionService conversionService, TypeDescriptor descriptor) {
265-
266-
return source -> {
267-
268-
if (String.class.isInstance(source)) {
269-
return (String) source;
270-
}
271-
272-
Object result = conversionService.canConvert(descriptor, STRING_DESCRIPTOR)
273-
? conversionService.convert(source, descriptor, STRING_DESCRIPTOR)
274-
: source == null ? null : source.toString();
275-
276-
if (result == null) {
277-
throw new IllegalArgumentException(String.format("Conversion of value %s resulted in null!", source));
278-
}
279-
280-
return (String) result;
281-
};
282-
}
283-
284267
private static class HandlerMethodParameters {
285268

286269
private static final List<Class<? extends Annotation>> ANNOTATIONS = Arrays.asList(RequestParam.class,
@@ -419,7 +402,7 @@ public String getVariableName() {
419402
return variableName;
420403
}
421404

422-
public Object prepareValue(Object value, ConversionService conversionService) {
405+
public Object prepareValue(Object value, FormatterFactory conversionService) {
423406

424407
Object result = prepareValue(value, conversionService, typeDescriptor);
425408

@@ -428,7 +411,7 @@ public Object prepareValue(Object value, ConversionService conversionService) {
428411

429412
@Nullable
430413
@SuppressWarnings("unchecked")
431-
public static Object prepareValue(@Nullable Object value, ConversionService conversionService,
414+
public static Object prepareValue(@Nullable Object value, FormatterFactory factory,
432415
@Nullable TypeDescriptor descriptor) {
433416

434417
if (descriptor == null || value == null) {
@@ -437,14 +420,18 @@ public static Object prepareValue(@Nullable Object value, ConversionService conv
437420

438421
value = ObjectUtils.unwrapOptional(value);
439422

423+
if (String.class.isInstance(value)) {
424+
return value;
425+
}
426+
440427
if (Collection.class.isInstance(value)) {
441428

442429
List<Object> prepared = new ArrayList<>();
443430

444431
for (Object element : (Collection<?>) value) {
445432

446433
TypeDescriptor elementTypeDescriptor = descriptor.elementTypeDescriptor(element);
447-
prepared.add(prepareValue(element, conversionService, elementTypeDescriptor));
434+
prepared.add(prepareValue(element, factory, elementTypeDescriptor));
448435
}
449436

450437
return prepared;
@@ -459,14 +446,14 @@ public static Object prepareValue(@Nullable Object value, ConversionService conv
459446
TypeDescriptor keyTypeDescriptor = descriptor.getMapKeyTypeDescriptor(entry.getKey());
460447
TypeDescriptor elementTypeDescriptor = descriptor.elementTypeDescriptor(entry.getValue());
461448

462-
prepared.put(prepareValue(entry.getKey(), conversionService, keyTypeDescriptor),
463-
prepareValue(entry.getValue(), conversionService, elementTypeDescriptor));
449+
prepared.put(prepareValue(entry.getKey(), factory, keyTypeDescriptor),
450+
prepareValue(entry.getValue(), factory, elementTypeDescriptor));
464451
}
465452

466453
return prepared;
467454
}
468455

469-
return getFormatter(conversionService, descriptor).apply(value);
456+
return factory.getFormatter(descriptor).apply(value);
470457
}
471458

472459
private String determineVariableName() {
@@ -508,6 +495,60 @@ public Object getVerifiedValue(Object[] values) {
508495
public abstract boolean isRequired();
509496
}
510497

498+
/**
499+
* Factory to create to-{@link String} converters by type. Caching, to avoid repeated calculations and
500+
* {@link Function} object creation.
501+
*
502+
* @author Oliver Drotbohm
503+
*/
504+
private static class FormatterFactory {
505+
506+
private static final Function<Object, String> DEFAULT = source -> source == null ? null : source.toString();
507+
508+
private final Map<TypeDescriptor, Function<Object, String>> formatters = new HashMap<>();
509+
private final ConversionService conversionService;
510+
511+
/**
512+
* Creates a new {@link FormatterFactory} for the given {@link ConversionService}.
513+
*
514+
* @param conversionService must not be {@literal null}.
515+
*/
516+
public FormatterFactory(ConversionService conversionService) {
517+
this.conversionService = conversionService;
518+
}
519+
520+
/**
521+
* Return the formatting function to map objects of the given {@link TypeDescriptor} to String.
522+
*
523+
* @param descriptor must not be {@literal null}.
524+
* @return will never be {@literal null}.
525+
*/
526+
public Function<Object, String> getFormatter(TypeDescriptor descriptor) {
527+
528+
if (STRING_DESCRIPTOR.equals(descriptor)) {
529+
return DEFAULT;
530+
}
531+
532+
return formatters.computeIfAbsent(descriptor, it -> {
533+
534+
if (!conversionService.canConvert(descriptor, STRING_DESCRIPTOR)) {
535+
return DEFAULT;
536+
}
537+
538+
return source -> {
539+
540+
Object result = conversionService.convert(source, descriptor, STRING_DESCRIPTOR);
541+
542+
if (result == null) {
543+
throw new IllegalArgumentException(String.format("Conversion of value %s resulted in null!", source));
544+
}
545+
546+
return (String) result;
547+
};
548+
});
549+
}
550+
}
551+
511552
/**
512553
* {@link HandlerMethodParameter} implementation to work with {@link RequestParam}.
513554
*

‎src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilderFactory.java

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -152,21 +152,25 @@ public WebMvcLinkBuilder linkTo(Object invocationValue) {
152152
return WebHandler.linkTo(invocationValue, WebMvcLinkBuilder::new, (builder, invocation) -> {
153153

154154
String[] primaryParams = SpringAffordanceBuilder.DISCOVERER.getParams(invocation.getMethod());
155-
ParamsRequestCondition paramsRequestCondition = new ParamsRequestCondition(primaryParams);
156155

157-
for (NameValueExpression<String> expression : paramsRequestCondition.getExpressions()) {
156+
if (primaryParams.length > 0) {
158157

159-
if (expression.isNegated()) {
160-
continue;
161-
}
158+
ParamsRequestCondition paramsRequestCondition = new ParamsRequestCondition(primaryParams);
162159

163-
String value = expression.getValue();
160+
for (NameValueExpression<String> expression : paramsRequestCondition.getExpressions()) {
164161

165-
if (value == null) {
166-
continue;
167-
}
162+
if (expression.isNegated()) {
163+
continue;
164+
}
165+
166+
String value = expression.getValue();
168167

169-
builder.queryParam(expression.getName(), value);
168+
if (value == null) {
169+
continue;
170+
}
171+
172+
builder.queryParam(expression.getName(), value);
173+
}
170174
}
171175

172176
MethodParameters parameters = MethodParameters.of(invocation.getMethod());

0 commit comments

Comments
 (0)