Skip to content

Commit

Permalink
performance improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@patriot1burke
patriot1burke committed Jul 15, 2019
1 parent a2cac0d commit f948c45
Show file tree
Hide file tree
Showing 20 changed files with 376 additions and 119 deletions.
3 changes: 3 additions & 0 deletions resteasy-core-spi/src/main/java/org/jboss/resteasy/spi/ResourceInvoker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@ public interface ResourceInvoker
CompletionStage<? extends Response> invoke(HttpRequest request, HttpResponse response, Object target);

Method getMethod();

// optimizations
boolean hasProduces();
}
3 changes: 1 addition & 2 deletions resteasy-core/src/main/java/org/jboss/resteasy/core/AcceptHeaderByFileSuffixFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,10 @@ public void setLanguageMappings(Map<String, String> languageMappings)
@Override
public void filter(ContainerRequestContext requestContext) throws IOException
{
if (mediaTypeMappings == null && languageMappings == null)
if ((mediaTypeMappings == null || mediaTypeMappings.isEmpty()) && (languageMappings == null || languageMappings.isEmpty()))
{
return;
}

URI uri = requestContext.getUriInfo().getRequestUri();
String rawPath = uri.getRawPath();
int lastSegment = rawPath.lastIndexOf('/');
Expand Down
9 changes: 9 additions & 0 deletions resteasy-core/src/main/java/org/jboss/resteasy/core/ResourceLocatorInvoker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.jboss.resteasy.util.GetRestful;

import javax.ws.rs.NotFoundException;
import javax.ws.rs.Produces;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
Expand All @@ -34,6 +35,7 @@ public class ResourceLocatorInvoker implements ResourceInvoker
protected ResteasyProviderFactory providerFactory;
protected ResourceLocator method;
protected ConcurrentHashMap<Class<?>, LocatorRegistry> cachedSubresources = new ConcurrentHashMap<Class<?>, LocatorRegistry>();
protected final boolean hasProduces;

public ResourceLocatorInvoker(final ResourceFactory resource, final InjectorFactory injector, final ResteasyProviderFactory providerFactory, final ResourceLocator locator)
{
Expand All @@ -42,8 +44,15 @@ public ResourceLocatorInvoker(final ResourceFactory resource, final InjectorFact
this.providerFactory = providerFactory;
this.method = locator;
this.methodInjector = injector.createMethodInjector(locator, providerFactory);
hasProduces = method.getMethod().isAnnotationPresent(Produces.class) || method.getMethod().getClass().isAnnotationPresent(Produces.class);
}

@Override
public boolean hasProduces() {
return hasProduces;
}


protected CompletionStage<Object> createResource(HttpRequest request, HttpResponse response)
{
return this.resource.createResource(request, response, providerFactory)
Expand Down
10 changes: 10 additions & 0 deletions resteasy-core/src/main/java/org/jboss/resteasy/core/ResourceMethodInvoker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.jboss.resteasy.util.DynamicFeatureContextDelegate;

import javax.ws.rs.ProcessingException;
import javax.ws.rs.Produces;
import javax.ws.rs.container.ContainerRequestFilter;
import javax.ws.rs.container.ContainerResponseFilter;
import javax.ws.rs.container.DynamicFeature;
Expand Down Expand Up @@ -86,6 +87,9 @@ public class ResourceMethodInvoker implements ResourceInvoker, JaxrsInterceptorR
protected ResourceInfo resourceInfo;

protected boolean expectsBody;
protected final boolean hasProduces;





Expand Down Expand Up @@ -167,6 +171,12 @@ protected void initializeUtils()
isSse = true;
method.markAsynchronous();
}
hasProduces = method.getMethod().isAnnotationPresent(Produces.class) || method.getMethod().getClass().isAnnotationPresent(Produces.class);
}

@Override
public boolean hasProduces() {
return hasProduces;
}

// spec section 9.3 Server API:
Expand Down
4 changes: 4 additions & 0 deletions resteasy-core/src/main/java/org/jboss/resteasy/core/registry/Expression.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ protected static int groupCount(String regex)
return groupCount;
}

public boolean isStatic() {
return groups.isEmpty();
}

public int getNumGroups()
{
return groups.size();
Expand Down
55 changes: 55 additions & 0 deletions resteasy-core/src/main/java/org/jboss/resteasy/core/registry/MatchCache.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package org.jboss.resteasy.core.registry;


import org.jboss.resteasy.specimpl.ResteasyUriInfo;
import org.jboss.resteasy.spi.HttpRequest;
import org.jboss.resteasy.spi.ResourceInvoker;

import javax.ws.rs.core.MediaType;
import java.util.List;
import java.util.Objects;

public class MatchCache {
public MediaType chosen;
public SegmentNode.Match match;
public ResourceInvoker invoker;

public static class Key {
public String path;
public int start;
public String method;
public MediaType contentType;
public List<MediaType> accepts;

public Key(final HttpRequest request, final int start) {
String matchingPath = ((ResteasyUriInfo) request.getUri()).getMatchingPath();
this.path = start == 0 ? matchingPath : matchingPath.substring(start);
this.start = start;
this.method = request.getHttpMethod();
this.contentType = request.getHttpHeaders().getMediaType();
this.accepts = request.getHttpHeaders().getAcceptableMediaTypes();
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Key key = (Key) o;
boolean b = start == key.start &&
path.equals(key.path) &&
method.equals(key.method) &&
Objects.equals(contentType, key.contentType);

This comment has been minimized.

Sign in to view

Copy link
@dmak

dmak Jul 10, 2020

The incoming request with content type with random boundary like this:

Content-type: multipart/form-data;boundary=bTHq6EqLI8B4NCLpr_stfilQniLLQOGKlJ

causes equals() be false all the time which allows the cache to grow infinitely, see RESTEASY-2646 for more details about the issue.

This comment has been minimized.

Sign in to view

Copy link
@patriot1burke

patriot1burke via email Jul 13, 2020

Author Collaborator

This comment has been minimized.

Sign in to view

Copy link
@asoldano

asoldano Jul 13, 2020

Member

Hi @patriot1burke , we have #2471 from @jimma , can you have a look please?
if (!b) return false;
if (accepts.isEmpty() && key.accepts.isEmpty()) return true;
if (accepts.size() != key.accepts.size()) return false;
// todo improve this
return b &&
accepts.equals(key.accepts);
}

@Override
public int hashCode() {
return Objects.hash(path, start, method);
}
}
}
2 changes: 1 addition & 1 deletion resteasy-core/src/main/java/org/jboss/resteasy/core/registry/MethodExpression.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public MethodExpression(final SegmentNode parent, final String segment, final Re
this.invoker = invoker;
}

public void populatePathParams(HttpRequest request, Matcher matcher, String path)
public void populatePathParams(HttpRequest request, Matcher matcher, String path)
{
ResteasyUriInfo uriInfo = (ResteasyUriInfo) request.getUri();
for (Group group : groups)
Expand Down
24 changes: 23 additions & 1 deletion resteasy-core/src/main/java/org/jboss/resteasy/core/registry/RootNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
import java.lang.reflect.Method;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import static org.jboss.resteasy.core.registry.SegmentNode.RESTEASY_CHOSEN_ACCEPT;

/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
Expand All @@ -19,6 +22,7 @@ public class RootNode
protected SegmentNode root = new SegmentNode("");
protected int size = 0;
protected MultivaluedMap<String, MethodExpression> bounded = new MultivaluedHashMap<String, MethodExpression>();
protected ConcurrentHashMap<MatchCache.Key, MatchCache> cache = new ConcurrentHashMap<>();

public int getSize()
{
Expand All @@ -38,9 +42,27 @@ public MultivaluedMap<String, ResourceInvoker> getBounded()
return rtn;
}

private static boolean CACHE = true;

public ResourceInvoker match(HttpRequest request, int start)
{
return root.match(request, start);
if (!CACHE) {
return root.match(request, start).invoker;
}
MatchCache.Key key = new MatchCache.Key(request, start);
MatchCache match = cache.get(key);
if (match != null) {
//System.out.println("*** cache hit: " + key.method + " " + key.path);
request.setAttribute(RESTEASY_CHOSEN_ACCEPT, match.chosen);
} else {
match = root.match(request, start);
if (match.match != null && match.match.expression.getNumGroups() == 0 && match.invoker instanceof ResourceMethodInvoker) {
//System.out.println("*** caching: " + key.method + " " + key.path);
match.match = null;
cache.putIfAbsent(key, match);
}
}
return match.invoker;
}

public void removeBinding(String path, Method method)
Expand Down
52 changes: 17 additions & 35 deletions resteasy-core/src/main/java/org/jboss/resteasy/core/registry/SegmentNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@
import javax.ws.rs.NotAllowedException;
import javax.ws.rs.NotFoundException;
import javax.ws.rs.NotSupportedException;
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.ResponseBuilder;

import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -73,7 +71,7 @@ public Match(final MethodExpression expression, final Matcher matcher)
}
}

public ResourceInvoker match(HttpRequest request, int start)
public MatchCache match(HttpRequest request, int start)
{
String path = ((ResteasyUriInfo) request.getUri()).getMatchingPath();
RESTEasyTracingLogger logger = RESTEasyTracingLogger.getInstance(request);
Expand Down Expand Up @@ -104,6 +102,8 @@ public ResourceInvoker match(HttpRequest request, int start)
ResourceInvoker invoker = expression.getInvoker();
if (invoker instanceof ResourceLocatorInvoker)
{
MatchCache ctx = new MatchCache();
ctx.invoker = invoker;
ResteasyUriInfo uriInfo = (ResteasyUriInfo) request.getUri();
int length = matcher.start(expression.getNumGroups() + 1);
if (length == -1)
Expand Down Expand Up @@ -135,7 +135,7 @@ public ResourceInvoker match(HttpRequest request, int start)
}
expression.populatePathParams(request, matcher, path);
logger.log("MATCH_LOCATOR", invoker.getMethod());
return invoker;
return ctx;
}
else
{
Expand All @@ -150,10 +150,10 @@ public ResourceInvoker match(HttpRequest request, int start)
{
throw new NotFoundException(Messages.MESSAGES.couldNotFindResourceForFullPath(request.getUri().getRequestUri()));
}
Match match = match(matches, request.getHttpMethod(), request);
match.expression.populatePathParams(request, match.matcher, path);
logger.log("MATCH_PATH_SELECTED", match.expression.getRegex());
return match.expression.getInvoker();
MatchCache match = match(matches, request.getHttpMethod(), request);
match.match.expression.populatePathParams(request, match.match.matcher, path);
logger.log("MATCH_PATH_SELECTED", match.match.expression.getRegex());
return match;

}

Expand Down Expand Up @@ -302,29 +302,7 @@ public MediaType getAcceptType()
|| "qs".equals(name)) continue;
params.put(name, entry.getValue());
}
Annotation[] annotations = match.expression.invoker.getMethod().getAnnotations();
boolean hasProduces = false;
for (Annotation annotation : annotations)
{
if (annotation instanceof Produces)
{
hasProduces = true;
break;
}
}
if (!hasProduces)
{
annotations = match.expression.invoker.getMethod().getClass().getAnnotations();
for (Annotation annotation : annotations)
{
if (annotation instanceof Produces)
{
hasProduces = true;
break;
}
}
}
if (hasProduces)
if (match.expression.invoker.hasProduces())
{
params.put(RESTEASY_SERVER_HAS_PRODUCES, "true");
}
Expand Down Expand Up @@ -373,7 +351,7 @@ public int compareTo(SortEntry o)
}
}

public Match match(List<Match> matches, String httpMethod, HttpRequest request)
public MatchCache match(List<Match> matches, String httpMethod, HttpRequest request)
{
MediaType contentType = request.getHttpHeaders().getMediaType();

Expand Down Expand Up @@ -511,7 +489,6 @@ else if (!consumeMatch)

for (SortFactor consume : consumeCombo)
{
final Method m = match.expression.getInvoker().getMethod();
sortList.add(new SortEntry(match, consume, sortFactor, produce));
}
}
Expand All @@ -528,8 +505,13 @@ else if (!consumeMatch)
{
LogMessages.LOGGER.multipleMethodsMatch(requestToString(request), mm);
}
request.setAttribute(RESTEASY_CHOSEN_ACCEPT, sortEntry.getAcceptType());
return sortEntry.match;
MediaType acceptType = sortEntry.getAcceptType();
request.setAttribute(RESTEASY_CHOSEN_ACCEPT, acceptType);
MatchCache ctx = new MatchCache();
ctx.chosen = acceptType;
ctx.match = sortEntry.match;
ctx.invoker = sortEntry.match.expression.invoker;
return ctx;
}

protected void addExpression(MethodExpression expression)
Expand Down

0 comments on commit f948c45

Please sign in to comment.