Avoid using regex matching for static patterns
Prior to this commit (and the previous one), the `AntPathStringMatcher` (inner class of `AntPathmatcher`) would compile `Pattern` instances and use regex matching even for static patterns such as `"/home"`. This change introduces a shortcut in the string matcher algorithm to skip the `Pattern` creation and uses `String` equality instead. Static patterns are quite common in applications and this change can bring performance improvements, depending on the mix of patterns configured in the web application. In benchmarks (added with this commit), we're seeing +20% throughput and -40% allocation. This of course can vary depending on the number of static patterns configured in the application. Closes gh-24887
This commit is contained in:
Brian Clozel
parent
9bfe410a0c
commit
567265123b
|
|
@ -80,7 +80,7 @@ public class AntPathMatcher implements PathMatcher {
|
|||
|
||||
private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\{[^/]+?}");
|
||||
|
||||
private static final char[] WILDCARD_CHARS = { '*', '?', '{' };
|
||||
private static final char[] WILDCARD_CHARS = {'*', '?', '{'};
|
||||
|
||||
|
||||
private String pathSeparator;
|
||||
|
|
@ -646,20 +646,15 @@ public class AntPathMatcher implements PathMatcher {
|
|||
|
||||
private static final String DEFAULT_VARIABLE_PATTERN = "(.*)";
|
||||
|
||||
private final String rawPattern;
|
||||
|
||||
private final boolean caseSensitive;
|
||||
|
||||
private final boolean exactMatch;
|
||||
|
||||
@Nullable
|
||||
private final Pattern pattern;
|
||||
|
||||
private final String originalPattern;
|
||||
|
||||
/**
|
||||
* True if this matcher accepts any value (e.g. regex <code>.*</code>)
|
||||
*/
|
||||
private final boolean acceptAny;
|
||||
|
||||
/**
|
||||
* True if given pattern is a literal pattern, thus candidate string may be tested by strings comparison for better perf.
|
||||
*/
|
||||
private final boolean allowStringComparison;
|
||||
|
||||
private final List<String> variableNames = new LinkedList<>();
|
||||
|
||||
public AntPathStringMatcher(String pattern) {
|
||||
|
|
@ -667,6 +662,8 @@ public class AntPathMatcher implements PathMatcher {
|
|||
}
|
||||
|
||||
public AntPathStringMatcher(String pattern, boolean caseSensitive) {
|
||||
this.rawPattern = pattern;
|
||||
this.caseSensitive = caseSensitive;
|
||||
StringBuilder patternBuilder = new StringBuilder();
|
||||
Matcher matcher = GLOB_PATTERN.matcher(pattern);
|
||||
int end = 0;
|
||||
|
|
@ -696,13 +693,17 @@ public class AntPathMatcher implements PathMatcher {
|
|||
}
|
||||
end = matcher.end();
|
||||
}
|
||||
patternBuilder.append(quote(pattern, end, pattern.length()));
|
||||
this.pattern = (caseSensitive ? Pattern.compile(patternBuilder.toString()) :
|
||||
Pattern.compile(patternBuilder.toString(), Pattern.CASE_INSENSITIVE));
|
||||
|
||||
this.originalPattern = pattern;
|
||||
this.acceptAny = this.pattern.pattern().equals(DEFAULT_VARIABLE_PATTERN);
|
||||
this.allowStringComparison = end == 0;
|
||||
// No glob pattern was found, this is an exact String match
|
||||
if (end == 0) {
|
||||
this.exactMatch = true;
|
||||
this.pattern = null;
|
||||
}
|
||||
else {
|
||||
this.exactMatch = false;
|
||||
patternBuilder.append(quote(pattern, end, pattern.length()));
|
||||
this.pattern = (this.caseSensitive ? Pattern.compile(patternBuilder.toString()) :
|
||||
Pattern.compile(patternBuilder.toString(), Pattern.CASE_INSENSITIVE));
|
||||
}
|
||||
}
|
||||
|
||||
private String quote(String s, int start, int end) {
|
||||
|
|
@ -717,43 +718,29 @@ public class AntPathMatcher implements PathMatcher {
|
|||
* @return {@code true} if the string matches against the pattern, or {@code false} otherwise.
|
||||
*/
|
||||
public boolean matchStrings(String str, @Nullable Map<String, String> uriTemplateVariables) {
|
||||
// Check whether current pattern accepts any value or given string is exact match
|
||||
if (this.acceptAny) {
|
||||
if (uriTemplateVariables != null) {
|
||||
assertUrlVariablesCount(1);
|
||||
uriTemplateVariables.put(this.variableNames.get(0), str);
|
||||
}
|
||||
return true;
|
||||
if (this.exactMatch) {
|
||||
return this.caseSensitive ? this.rawPattern.equals(str) : this.rawPattern.equalsIgnoreCase(str);
|
||||
}
|
||||
else if (this.allowStringComparison && str.equals(this.originalPattern)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
Matcher matcher = this.pattern.matcher(str);
|
||||
if (matcher.matches()) {
|
||||
if (uriTemplateVariables != null) {
|
||||
assertUrlVariablesCount(matcher.groupCount());
|
||||
for (int i = 1; i <= matcher.groupCount(); i++) {
|
||||
String name = this.variableNames.get(i - 1);
|
||||
String value = matcher.group(i);
|
||||
uriTemplateVariables.put(name, value);
|
||||
else if (this.pattern != null) {
|
||||
Matcher matcher = this.pattern.matcher(str);
|
||||
if (matcher.matches()) {
|
||||
if (uriTemplateVariables != null) {
|
||||
if (this.variableNames.size() != matcher.groupCount()) {
|
||||
throw new IllegalArgumentException("The number of capturing groups in the pattern segment " +
|
||||
this.pattern + " does not match the number of URI template variables it defines, " +
|
||||
"which can occur if capturing groups are used in a URI template regex. " +
|
||||
"Use non-capturing groups instead.");
|
||||
}
|
||||
for (int i = 1; i <= matcher.groupCount(); i++) {
|
||||
String name = this.variableNames.get(i - 1);
|
||||
String value = matcher.group(i);
|
||||
uriTemplateVariables.put(name, value);
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
else {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// SPR-8455
|
||||
private void assertUrlVariablesCount(int expected) {
|
||||
if (this.variableNames.size() != expected) {
|
||||
throw new IllegalArgumentException("The number of capturing groups in the pattern segment " +
|
||||
this.pattern + " does not match the number of URI template variables it defines, " +
|
||||
"which can occur if capturing groups are used in a URI template regex. " +
|
||||
"Use non-capturing groups instead.");
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -130,9 +130,6 @@ class AntPathMatcherTests {
|
|||
assertThat(pathMatcher.match("", "")).isTrue();
|
||||
|
||||
assertThat(pathMatcher.match("/{bla}.*", "/testing.html")).isTrue();
|
||||
|
||||
// Test that sending the same pattern will not match (gh #24887)
|
||||
assertThat(pathMatcher.match("/bla/{foo:[0-9]}", "/bla/{foo:[0-9]}")).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
|
|
@ -0,0 +1,278 @@
|
|||
/*
|
||||
* Copyright 2002-2020 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.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.web.util.pattern;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
|
||||
import org.openjdk.jmh.annotations.Benchmark;
|
||||
import org.openjdk.jmh.annotations.BenchmarkMode;
|
||||
import org.openjdk.jmh.annotations.Level;
|
||||
import org.openjdk.jmh.annotations.Mode;
|
||||
import org.openjdk.jmh.annotations.Scope;
|
||||
import org.openjdk.jmh.annotations.Setup;
|
||||
import org.openjdk.jmh.annotations.State;
|
||||
import org.openjdk.jmh.infra.Blackhole;
|
||||
|
||||
import org.springframework.http.server.PathContainer;
|
||||
import org.springframework.util.AntPathMatcher;
|
||||
|
||||
/**
|
||||
* Benchmarks for matching requests paths against path patterns in a web context.
|
||||
* We're considering here the {@link org.springframework.util.AntPathMatcher} and
|
||||
* {@link PathPatternParser} implementations with typical sets of patterns.
|
||||
* @author Brian Clozel
|
||||
*/
|
||||
@BenchmarkMode(Mode.Throughput)
|
||||
public class PathMatchingBenchmark {
|
||||
|
||||
@State(Scope.Benchmark)
|
||||
public static class AllRoutesPatternParser extends PatternParserData {
|
||||
|
||||
@Setup(Level.Trial)
|
||||
public void registerPatterns() {
|
||||
parseRoutes(RouteGenerator.allRoutes());
|
||||
}
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
public void matchAllRoutesWithPathPatternParser(AllRoutesPatternParser data, Blackhole bh) {
|
||||
for (PathContainer path : data.requestPaths) {
|
||||
for (PathPattern pattern : data.patterns) {
|
||||
bh.consume(pattern.matches(path));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
public void matchAndSortAllRoutesWithPathPatternParser(AllRoutesPatternParser data, Blackhole bh) {
|
||||
for (PathContainer path : data.requestPaths) {
|
||||
List<PathPattern> matches = new ArrayList<>();
|
||||
for (PathPattern pattern : data.patterns) {
|
||||
if (pattern.matches(path)) {
|
||||
matches.add(pattern);
|
||||
}
|
||||
}
|
||||
Collections.sort(matches);
|
||||
bh.consume(matches);
|
||||
}
|
||||
}
|
||||
|
||||
@State(Scope.Benchmark)
|
||||
public static class StaticRoutesPatternParser extends PatternParserData {
|
||||
|
||||
@Setup(Level.Trial)
|
||||
public void registerPatterns() {
|
||||
parseRoutes(RouteGenerator.staticRoutes());
|
||||
}
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
public void matchStaticRoutesWithPathPatternParser(StaticRoutesPatternParser data, Blackhole bh) {
|
||||
for (PathContainer path : data.requestPaths) {
|
||||
for (PathPattern pattern : data.patterns) {
|
||||
bh.consume(pattern.matches(path));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@State(Scope.Benchmark)
|
||||
public static class AllRoutesAntPathMatcher extends AntPathMatcherData {
|
||||
|
||||
@Setup(Level.Trial)
|
||||
public void registerPatterns() {
|
||||
parseRoutes(RouteGenerator.allRoutes());
|
||||
}
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
public void matchAllRoutesWithAntPathMatcher(AllRoutesAntPathMatcher data, Blackhole bh) {
|
||||
for (String path : data.requestPaths) {
|
||||
for (String pattern : data.patterns) {
|
||||
bh.consume(data.matcher.match(pattern, path));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
public void matchAndSortAllRoutesWithAntPathMatcher(AllRoutesAntPathMatcher data, Blackhole bh) {
|
||||
for (String path : data.requestPaths) {
|
||||
List<String> matches = new ArrayList<>();
|
||||
for (String pattern : data.patterns) {
|
||||
if (data.matcher.match(pattern, path)) {
|
||||
matches.add(pattern);
|
||||
}
|
||||
}
|
||||
matches.sort(data.matcher.getPatternComparator(path));
|
||||
bh.consume(matches);
|
||||
}
|
||||
}
|
||||
|
||||
@State(Scope.Benchmark)
|
||||
public static class StaticRoutesAntPathMatcher extends AntPathMatcherData {
|
||||
|
||||
@Setup(Level.Trial)
|
||||
public void registerPatterns() {
|
||||
parseRoutes(RouteGenerator.staticRoutes());
|
||||
}
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
public void matchStaticRoutesWithAntPathMatcher(StaticRoutesAntPathMatcher data, Blackhole bh) {
|
||||
for (String path : data.requestPaths) {
|
||||
for (String pattern : data.patterns) {
|
||||
bh.consume(data.matcher.match(pattern, path));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
static class PatternParserData {
|
||||
|
||||
List<PathPattern> patterns = new ArrayList<>();
|
||||
|
||||
List<PathContainer> requestPaths = new ArrayList<>();
|
||||
|
||||
void parseRoutes(List<Route> routes) {
|
||||
PathPatternParser parser = new PathPatternParser();
|
||||
routes.forEach(route -> {
|
||||
this.patterns.add(parser.parse(route.pattern));
|
||||
route.matchingPaths.forEach(path -> this.requestPaths.add(PathContainer.parsePath(path)));
|
||||
});
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
static class AntPathMatcherData {
|
||||
|
||||
AntPathMatcher matcher = new AntPathMatcher();
|
||||
|
||||
List<String> patterns = new ArrayList<>();
|
||||
|
||||
List<String> requestPaths = new ArrayList<>();
|
||||
|
||||
void parseRoutes(List<Route> routes) {
|
||||
routes.forEach(route -> {
|
||||
this.patterns.add(route.pattern);
|
||||
this.requestPaths.addAll(route.matchingPaths);
|
||||
});
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
* Route in the web application.
|
||||
* Each route has a path pattern and can generate sets of matching request paths for that pattern.
|
||||
*/
|
||||
static class Route {
|
||||
|
||||
private final String pattern;
|
||||
|
||||
private final List<String> matchingPaths;
|
||||
|
||||
public Route(String pattern, String... matchingPaths) {
|
||||
this.pattern = pattern;
|
||||
if (matchingPaths.length > 0) {
|
||||
this.matchingPaths = Arrays.asList(matchingPaths);
|
||||
}
|
||||
else {
|
||||
this.matchingPaths = Collections.singletonList(pattern);
|
||||
}
|
||||
}
|
||||
|
||||
public String pattern() {
|
||||
return this.pattern;
|
||||
}
|
||||
|
||||
public Iterable<String> matchingPaths() {
|
||||
return this.matchingPaths;
|
||||
}
|
||||
}
|
||||
|
||||
static class RouteGenerator {
|
||||
|
||||
static List<Route> staticRoutes() {
|
||||
return Arrays.asList(
|
||||
new Route("/"),
|
||||
new Route("/why-spring"),
|
||||
new Route("/microservices"),
|
||||
new Route("/reactive"),
|
||||
new Route("/event-driven"),
|
||||
new Route("/cloud"),
|
||||
new Route("/web-applications"),
|
||||
new Route("/serverless"),
|
||||
new Route("/batch"),
|
||||
new Route("/community/overview"),
|
||||
new Route("/community/team"),
|
||||
new Route("/community/events"),
|
||||
new Route("/community/support"),
|
||||
new Route("/some/other/section"),
|
||||
new Route("/blog.atom")
|
||||
);
|
||||
}
|
||||
|
||||
static List<Route> captureRoutes() {
|
||||
return Arrays.asList(
|
||||
new Route("/guides"),
|
||||
new Route("/guides/gs/{repositoryName}",
|
||||
"/guides/gs/rest-service", "/guides/gs/scheduling-tasks",
|
||||
"/guides/gs/consuming-rest", "/guides/gs/relational-data-access"),
|
||||
new Route("/projects"),
|
||||
new Route("/projects/{name}",
|
||||
"/projects/spring-boot", "/projects/spring-framework",
|
||||
"/projects/spring-data", "/projects/spring-security", "/projects/spring-cloud"),
|
||||
new Route("/blog/category/{category}.atom",
|
||||
"/blog/category/releases.atom", "/blog/category/engineering.atom",
|
||||
"/blog/category/news.atom"),
|
||||
new Route("/tools/{name}", "/tools/eclipse", "/tools/vscode"),
|
||||
new Route("/team/{username}",
|
||||
"/team/jhoeller", "/team/bclozel", "/team/snicoll", "/team/sdeleuze", "/team/rstoyanchev"),
|
||||
new Route("/api/projects/{projectId}",
|
||||
"/api/projects/spring-boot", "/api/projects/spring-framework",
|
||||
"/api/projects/reactor", "/api/projects/spring-data",
|
||||
"/api/projects/spring-restdocs", "/api/projects/spring-batch"),
|
||||
new Route("/api/projects/{projectId}/releases/{version}",
|
||||
"/api/projects/spring-boot/releases/2.3.0", "/api/projects/spring-framework/releases/5.3.0",
|
||||
"/api/projects/spring-boot/releases/2.2.0", "/api/projects/spring-framework/releases/5.2.0")
|
||||
);
|
||||
}
|
||||
|
||||
static List<Route> regexRoute() {
|
||||
return Arrays.asList(
|
||||
new Route("/blog/{year:\\\\d+}/{month:\\\\d+}/{day:\\\\d+}/{slug}",
|
||||
"/blog/2020/01/01/spring-boot-released", "/blog/2020/02/10/this-week-in-spring",
|
||||
"/blog/2020/03/12/spring-one-conference-2020", "/blog/2020/05/17/spring-io-barcelona-2020",
|
||||
"/blog/2020/05/17/spring-io-barcelona-2020", "/blog/2020/06/06/spring-cloud-release"),
|
||||
new Route("/user/{name:[a-z]+}",
|
||||
"/user/emily", "/user/example", "/user/spring")
|
||||
);
|
||||
}
|
||||
|
||||
static List<Route> allRoutes() {
|
||||
List<Route> routes = new ArrayList<>();
|
||||
routes.addAll(staticRoutes());
|
||||
routes.addAll(captureRoutes());
|
||||
routes.addAll(regexRoute());
|
||||
routes.add(new Route("/static/**", "/static/image.png", "/static/style.css"));
|
||||
routes.add(new Route("/**", "/notfound", "/favicon.ico"));
|
||||
return routes;
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
|
@ -8,7 +8,7 @@
|
|||
<suppress files="[\\/]src[\\/](test|testFixtures)[\\/]java[\\/]" checks="SpringJUnit5" message="should not be public" />
|
||||
|
||||
<!-- JMH benchmarks -->
|
||||
<suppress files="[\\/]src[\\/]jmh[\\/]java[\\/]org[\\/]springframework[\\/]" checks="JavadocVariable|JavadocStyle" />
|
||||
<suppress files="[\\/]src[\\/]jmh[\\/]java[\\/]org[\\/]springframework[\\/]" checks="JavadocVariable|JavadocStyle|InnerTypeLast" />
|
||||
|
||||
<!-- spring-beans -->
|
||||
<suppress files="TypeMismatchException" checks="MutableException"/>
|
||||
|
|
|
|||
Loading…
Reference in New Issue