Skip to content

Commit

Permalink
#1148 - Further performance improvements in link creation.
Browse files Browse the repository at this point in the history 
The implementation details of WebHandler have been significantly refactored to rather work with structures that allow better cacheability by clearly separating abstractions over the statically available information from the per-invocation aspects. This results in a new HandlerMethodParameter(s) abstraction within WebHandler. BoundMethodParameter has been removed entirely. HandlerMethodParameters are create once then cached for every controller method being linked to.

DummyInvocationUtils now creates a ThreadLocal cache of the proxies created for calls to methodOn(…) as they essentially only act as basis for subsequent calls to the methods on the proxy created which in turn are expected to be handed into a linkTo(…) call which obtains the invocation right away. This avoids overhead in cases methodOn(…) is called multiple times for the same controller from a single controller.

The lookup of the LastInvocationAware was previously routed through the proxy, handled by InvocationRecordingMethodInterceptor. This resulted in a second, reflective call for every link creation. DummyInvocationUtils now provides a dedicated lookup method as it knows about the structure of the proxy it created and thus can unfold the recorded invocation more effectively.

The LinkBuilder type hierarchy now works with UriComponents and only creates a UriComponentsBuilder if it needs to modify the backing link in the first place. This avoids superfluous back and forth between UriComponents and UriComponentsBuilders that involved quite a bit of String parsing and creation.

EncodingUtils now starts from a StandardCharsets.UTF_8 to avoid repeated Charset creation.

The changes result in a ~3x performance compared to 1.0.2.RELEASE:

1.0.2.RELEASE

Benchmark                                         Mode  Cnt         Score        Error  Units
ControllerLinkBuilderBenchmark.noLinkCreation    thrpt   10  39004583,189 ± 751668,181  ops/s
ControllerLinkBuilderBenchmark.pureLinkCreation  thrpt   10     43443,133 ±    783,120  ops/s
ControllerLinkBuilderBenchmark.withLinkCreation  thrpt   10     60201,629 ±   1292,179  ops/s

1.1 / 1.0.3 SNAPSHOT

Benchmark                                         Mode  Cnt         Score        Error  Units
ControllerLinkBuilderBenchmark.noLinkCreation    thrpt   10  39618560,950 ± 612794,310  ops/s
ControllerLinkBuilderBenchmark.pureLinkCreation  thrpt   10    121700,634 ±   1510,415  ops/s
ControllerLinkBuilderBenchmark.withLinkCreation  thrpt   10    121982,085 ±   3344,206  ops/s

noLinkCreation - creates a single RepresentationModel instance but adds no links
pureLinkCreation - creates a single link pointing to a controller method
withLinkCreation - creates a single RepresentationModel instance adding a single link
  • Loading branch information
@odrotbohm
odrotbohm committed Dec 6, 2019
1 parent 9f05bea commit 452189e
Show file tree
Hide file tree
Showing 14 changed files with 382 additions and 350 deletions.
33 changes: 11 additions & 22 deletions src/main/java/org/springframework/hateoas/server/core/AnnotationAttribute.java
View file
Expand Up @@ -16,7 +16,6 @@
package org.springframework.hateoas.server.core;

import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;

import org.springframework.core.MethodParameter;
import org.springframework.core.annotation.AnnotationUtils;
Expand All @@ -25,7 +24,7 @@

/**
* Simply helper to reference a dedicated attribute of an {@link Annotation}.
*
*
* @author Oliver Gierke
*/
public class AnnotationAttribute {
Expand All @@ -35,7 +34,7 @@ public class AnnotationAttribute {

/**
* Creates a new {@link AnnotationAttribute} to the {@code value} attribute of the given {@link Annotation} type.
*
*
* @param annotationType must not be {@literal null}.
*/
public AnnotationAttribute(Class<? extends Annotation> annotationType) {
Expand All @@ -44,7 +43,7 @@ public AnnotationAttribute(Class<? extends Annotation> annotationType) {

/**
* Creates a new {@link AnnotationAttribute} for the given {@link Annotation} type and annotation attribute name.
*
*
* @param annotationType must not be {@literal null}.
* @param attributeName can be {@literal null}, defaults to {@code value}.
*/
Expand All @@ -58,7 +57,7 @@ public AnnotationAttribute(Class<? extends Annotation> annotationType, @Nullable

/**
* Returns the annotation type.
*
*
* @return the annotationType
*/
public Class<? extends Annotation> getAnnotationType() {
Expand All @@ -67,43 +66,33 @@ public Class<? extends Annotation> getAnnotationType() {

/**
* Reads the {@link Annotation} attribute's value from the given {@link MethodParameter}.
*
*
* @param parameter must not be {@literal null}.
* @return
*/
@Nullable
public String getValueFrom(MethodParameter parameter) {

Assert.notNull(parameter, "MethodParameter must not be null!");
Annotation annotation = parameter.getParameterAnnotation(annotationType);
return annotation == null ? null : getValueFrom(annotation);
}

/**
* Reads the {@link Annotation} attribute's value from the given {@link AnnotatedElement}.
*
* @param annotatedElement must not be {@literal null}.
* @return
*/
@Nullable
public String getValueFrom(AnnotatedElement annotatedElement) {
Annotation annotation = parameter.getParameterAnnotation(annotationType);

Assert.notNull(annotatedElement, "Annotated element must not be null!");
Annotation annotation = annotatedElement.getAnnotation(annotationType);
return annotation == null ? null : getValueFrom(annotation);
}

/**
* Returns the {@link Annotation} attribute's value from the given {@link Annotation}.
*
*
* @param annotation must not be {@literal null}.
* @return
*/
@Nullable
public String getValueFrom(Annotation annotation) {

Assert.notNull(annotation, "Annotation must not be null!");
return (String) (attributeName == null ? AnnotationUtils.getValue(annotation) : AnnotationUtils.getValue(
annotation, attributeName));

return (String) (attributeName == null //
? AnnotationUtils.getValue(annotation) //
: AnnotationUtils.getValue(annotation, attributeName));
}
}
46 changes: 31 additions & 15 deletions src/main/java/org/springframework/hateoas/server/core/DummyInvocationUtils.java
View file
Expand Up @@ -20,9 +20,12 @@

import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;

import org.aopalliance.intercept.MethodInterceptor;
import org.springframework.aop.framework.Advised;
import org.springframework.aop.framework.ProxyFactory;
import org.springframework.aop.target.EmptyTargetSource;
import org.springframework.lang.Nullable;
Expand All @@ -36,6 +39,8 @@
*/
public class DummyInvocationUtils {

private static final ThreadLocal<Map<CacheKey<?>, Object>> CACHE = ThreadLocal.withInitial(HashMap::new);

/**
* Method interceptor that records the last method invocation and creates a proxy for the return value that exposes
* the method invocation.
Expand All @@ -44,18 +49,10 @@ public class DummyInvocationUtils {
*/
private static class InvocationRecordingMethodInterceptor implements MethodInterceptor, LastInvocationAware {

private static final Method GET_INVOCATIONS;
private static final Method GET_OBJECT_PARAMETERS;

private final Class<?> targetType;
private final Object[] objectParameters;
private MethodInvocation invocation;

static {
GET_INVOCATIONS = ReflectionUtils.findMethod(LastInvocationAware.class, "getLastInvocation");
GET_OBJECT_PARAMETERS = ReflectionUtils.findMethod(LastInvocationAware.class, "getObjectParameters");
}

/**
* Creates a new {@link InvocationRecordingMethodInterceptor} carrying the given parameters forward that might be
* needed to populate the class level mapping.
Expand Down Expand Up @@ -83,11 +80,7 @@ public Object invoke(org.aopalliance.intercept.MethodInvocation invocation) {

Method method = invocation.getMethod();

if (GET_INVOCATIONS.equals(method)) {
return getLastInvocation();
} else if (GET_OBJECT_PARAMETERS.equals(method)) {
return getObjectParameters();
} else if (ReflectionUtils.isObjectMethod(method)) {
if (ReflectionUtils.isObjectMethod(method)) {
return ReflectionUtils.invokeMethod(method, invocation.getThis(), invocation.getArguments());
}

Expand Down Expand Up @@ -131,12 +124,29 @@ public Iterator<Object> getObjectParameters() {
* @param parameters parameters to extend template variables in the type level mapping.
* @return
*/
@SuppressWarnings("unchecked")
public static <T> T methodOn(Class<T> type, Object... parameters) {

Assert.notNull(type, "Given type must not be null!");

InvocationRecordingMethodInterceptor interceptor = new InvocationRecordingMethodInterceptor(type, parameters);
return getProxyWithInterceptor(type, interceptor, type.getClassLoader());
return (T) CACHE.get().computeIfAbsent(CacheKey.of(type, parameters), it -> {

InvocationRecordingMethodInterceptor interceptor = new InvocationRecordingMethodInterceptor(it.type,
it.arguments);
return getProxyWithInterceptor(it.type, interceptor, type.getClassLoader());
});
}

/**
* Returns the {@link LastInvocationAware} instance from the given source, that essentially has to be a proxy created
* via {@link #methodOn(Class, Object...)} and subsequent {@code linkTo(…)} calls.
*
* @param source must not be {@literal null}.
* @return
*/
@Nullable
public static LastInvocationAware getLastInvocationAware(Object source) {
return (LastInvocationAware) ((Advised) source).getAdvisors()[0].getAdvice();
}

@SuppressWarnings("unchecked")
Expand All @@ -157,6 +167,12 @@ private static <T> T getProxyWithInterceptor(Class<?> type, InvocationRecordingM
return (T) factory.getProxy(classLoader);
}

@Value(staticConstructor = "of")
private static class CacheKey<T> {
Class<T> type;
Object[] arguments;
}

@Value
private static class SimpleMethodInvocation implements MethodInvocation {

Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/springframework/hateoas/server/core/EncodingUtils.java
View file
Expand Up @@ -17,6 +17,9 @@

import lombok.experimental.UtilityClass;

import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;

import org.springframework.util.Assert;
import org.springframework.web.util.UriUtils;

Expand All @@ -31,7 +34,7 @@
@UtilityClass
class EncodingUtils {

private static final String ENCODING = "UTF-8";
private static final Charset ENCODING = StandardCharsets.UTF_8;

/**
* Encodes the given path value.
Expand Down
59 changes: 19 additions & 40 deletions src/main/java/org/springframework/hateoas/server/core/LinkBuilderSupport.java
View file
Expand Up @@ -25,7 +25,6 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.function.Function;

import org.springframework.hateoas.Affordance;
import org.springframework.hateoas.IanaLinkRelations;
Expand All @@ -49,34 +48,28 @@
*/
public abstract class LinkBuilderSupport<T extends LinkBuilder> implements LinkBuilder {

private final UriComponentsBuilder builder;
private final @Getter List<Affordance> affordances;

private UriComponents components;

/**
* Creates a new {@link LinkBuilderSupport} using the given {@link UriComponents}.
*
* @param builder must not be {@literal null}.
*/
protected LinkBuilderSupport(UriComponentsBuilder builder) {
protected LinkBuilderSupport(UriComponents builder) {
this(builder, Collections.emptyList());
}

protected LinkBuilderSupport(UriComponentsBuilder builder, List<Affordance> affordances) {

Assert.notNull(builder, "UriComponentsBuilder must not be null!");
Assert.notNull(affordances, "Affordances must not be null!");

this.builder = builder.cloneBuilder();
this.affordances = affordances;
}

protected LinkBuilderSupport(UriComponents components, List<Affordance> affordances) {

Assert.notNull(components, "UriComponents must not be null!");
Assert.notNull(affordances, "Affordances must not be null!");

this.builder = UriComponentsBuilder.fromUriString(components.toUriString());
// this.builder = UriComponentsBuilder.newInstance().uriComponents(components);
this.affordances = affordances;

this.components = components;
}

/*
Expand Down Expand Up @@ -108,28 +101,27 @@ public T slash(@Nullable Object object) {

protected T slash(UriComponents components, boolean encoded) {

return withFreshBuilder(builder -> {
UriComponentsBuilder builder = UriComponentsBuilder.newInstance().uriComponents(this.components);

for (String pathSegment : components.getPathSegments()) {
builder.pathSegment(encoded ? pathSegment : encodePath(pathSegment));
}
for (String pathSegment : components.getPathSegments()) {
builder.pathSegment(encoded ? pathSegment : encodePath(pathSegment));
}

String fragment = components.getFragment();
String fragment = components.getFragment();

if (fragment != null && !fragment.trim().isEmpty()) {
builder.fragment(encoded ? fragment : encodeFragment(fragment));
}
if (fragment != null && !fragment.trim().isEmpty()) {
builder.fragment(encoded ? fragment : encodeFragment(fragment));
}

return createNewInstance(builder.query(components.getQuery()), affordances);
});
return createNewInstance(builder.query(components.getQuery()).build(), affordances);
}

/*
* (non-Javadoc)
* @see org.springframework.hateoas.LinkBuilder#toUri()
*/
public URI toUri() {
return builder.build().toUri().normalize();
return components.toUri().normalize();
}

public T addAffordances(Collection<Affordance> affordances) {
Expand All @@ -138,7 +130,7 @@ public T addAffordances(Collection<Affordance> affordances) {
newAffordances.addAll(this.affordances);
newAffordances.addAll(affordances);

return createNewInstance(builder, newAffordances);
return createNewInstance(components, newAffordances);
}

/*
Expand All @@ -165,20 +157,7 @@ public Link withSelfRel() {
*/
@Override
public String toString() {
return builder.build().toUriString();
}

/**
* Executes the given {@link Function} using a freshly cloned {@link UriComponentsBuilder}.
*
* @param function must not be {@literal null}.
* @return
*/
protected <S> S withFreshBuilder(Function<UriComponentsBuilder, S> function) {

Assert.notNull(function, "Function must not be null!");

return function.apply(builder.cloneBuilder());
return components.toUriString();
}

/**
Expand All @@ -194,5 +173,5 @@ protected <S> S withFreshBuilder(Function<UriComponentsBuilder, S> function) {
* @param builder will never be {@literal null}.
* @return
*/
protected abstract T createNewInstance(UriComponentsBuilder builder, List<Affordance> affordances);
protected abstract T createNewInstance(UriComponents components, List<Affordance> affordances);
}
17 changes: 4 additions & 13 deletions ...java/org/springframework/hateoas/server/core/TemplateVariableAwareLinkBuilderSupport.java
View file
Expand Up @@ -20,7 +20,6 @@
import org.springframework.hateoas.Affordance;
import org.springframework.hateoas.TemplateVariables;
import org.springframework.web.util.UriComponents;
import org.springframework.web.util.UriComponentsBuilder;

/**
* A {@link LinkBuilderSupport} extension that can keep a list of {@link TemplateVariables} around.
Expand All @@ -32,14 +31,6 @@ public abstract class TemplateVariableAwareLinkBuilderSupport<T extends Template

private final TemplateVariables variables;

protected TemplateVariableAwareLinkBuilderSupport(UriComponentsBuilder builder, TemplateVariables variables,
List<Affordance> affordances) {

super(builder, affordances);

this.variables = variables;
}

protected TemplateVariableAwareLinkBuilderSupport(UriComponents components, TemplateVariables variables,
List<Affordance> affordances) {

Expand All @@ -50,14 +41,14 @@ protected TemplateVariableAwareLinkBuilderSupport(UriComponents components, Temp

/*
* (non-Javadoc)
* @see org.springframework.hateoas.core.LinkBuilderSupport#createNewInstance(org.springframework.web.util.UriComponentsBuilder, java.util.List)
* @see org.springframework.hateoas.server.core.LinkBuilderSupport#createNewInstance(org.springframework.web.util.UriComponents, java.util.List)
*/
@Override
protected final T createNewInstance(UriComponentsBuilder builder, List<Affordance> affordances) {
return createNewInstance(builder, affordances, variables);
protected final T createNewInstance(UriComponents components, List<Affordance> affordances) {
return createNewInstance(components, affordances, variables);
}

protected abstract T createNewInstance(UriComponentsBuilder builder, List<Affordance> affordances,
protected abstract T createNewInstance(UriComponents components, List<Affordance> affordances,
TemplateVariables variables);

/*
Expand Down

0 comments on commit 452189e

Please sign in to comment.