Repository navigation
Check the path condition first when request mappings are matched without a direct path match #37623
Description
When a request has no direct path match (any URL with a URI variable), AbstractHandlerMethodMapping.lookupHandlerMethod evaluates RequestMappingInfo.getMatchingCondition for every registration. That method checks the methods, params, headers, consumes, produces and version conditions before the path. With many mappings, almost every candidate pays for those checks and is only then rejected by the path. The method checks alone call request.getMethod() 3–4 times per mapping (method condition plus CorsUtils.isPreFlightRequest in the method, consumes and produces conditions), through the request wrappers Spring Security adds.
Production impact. Our Spring MVC app (Boot 4.2.0-RC1, Framework 7.1.0-RC1, Kotlin) has about 2,500 handler mappings, half with URI variables. A 3-hour JFR recording from production (settings=profile, 13,207 execution samples) shows:
AbstractHandlerMethodMapping.lookupHandlerMethod: 24.7% of all CPU samples, 64% of the samples underDispatcherServlet. Nearly all of it is inaddMatchingMappingsover all registrations.- Top self frames:
HttpServletRequestWrapper.getMethod9.5% (fromRequestMethodsRequestConditionandCorsUtils.isPreFlightRequest),HashMap$HashIterator.nextNode3.6% (iterating the registry), andPathPattern.hasLength/TreeSet.firstfromPathPatternsRequestCondition. - 38,783 requests in the window (median 3.3 ms), so the lookup accounts for very roughly 0.8 ms per request.
Proposed change (MVC and WebFlux, 4 files):
- In
RequestMappingInfo.getMatchingCondition, check the path condition(s) first. The other conditions have no side effects that depend on the order (ProducesRequestConditiononly caches the parsedAcceptheader, and that attribute is cleared inhandleMatch/handleNoMatch). The result of a match is unchanged; mappings with a non-matching path are simply rejected before the other conditions run. The custom condition stays last. - Make
MappingRegistry.registryaLinkedHashMap. The fallback iterates all registrations, and aHashMapkeySet iterator walks the empty buckets of the table. Iteration order also becomes registration order instead of hash order.
Benchmark. Our app's 439 controllers were loaded from the boot jar, and their mappings registered through RequestMappingHandlerMapping.getMappingForMethod + registerMapping (2,547 mappings). Each lookup goes through getHandlerInternal on a MockHttpServletRequest wrapped 3 times. Median µs per lookup, JDK 27, Apple M5 Max:
| Request | 7.1.0-RC1 | path first | + linked registry |
|---|---|---|---|
GET /rolle/{personReference:p2[0-9a-z]{7}}/{slug} (Host condition; 76% of our traffic) |
112 | 41 | 38 (−66%) |
GET /api/invoices/{id}/payment-links/{linkId} |
123 | 57 | 49 (−60%) |
PUT /api/invoices/{invoiceId}/voucher |
72 | 57 | 48 (−33%) |
| no matching mapping | 157 | 86 | 74 (−53%) |
| direct path match (unaffected) | 0.6 | 0.6 | 0.6 |
Every request resolved to the same handler method in all three variants.
Tests. On main (ed4e0f8) with the patch, :spring-webmvc:check passes (2,905 tests, checkstyle). :spring-webflux:test gives the same results as unpatched main: on my machine, three integration tests fail identically with and without the patch (DispatcherHandlerIntegrationTests, CoroutinesIntegrationTests and RequestMappingIntegrationTests fail with "failed to respond" for Reactor Netty/Jetty).
Possible follow-ups. With the path checked first, about a third of the remaining lookup time is ServletRequestPathUtils.getParsedRequestPath(request), which reads the parsed path from a request attribute (through the wrappers) once per mapping. Passing the parsed path down, or indexing registrations by their first literal path segment (similar to the trie idea in #24901), would remove most of the rest. Both are larger changes, so I left them out.
Apply. The patch is a plain diff against main that you can take into a PR in whatever form suits you. From the repository root:
curl -fsSL https://gist.githubusercontent.com/gregjotau/22f7137b392b771d5c74182cbfc641f2/raw/spring-mvc-lookup-path-first.patch | git applyDiff
diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java
index b2e9acf8ca..bc93aa3138 100644
--- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java
+++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java
@@ -22,7 +22,7 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -462,7 +462,8 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
*/
class MappingRegistry {
- private final Map<T, MappingRegistration<T>> registry = new HashMap<>();
+ // Linked for faster iteration: lookups without a direct path match iterate over all registrations.
+ private final Map<T, MappingRegistration<T>> registry = new LinkedHashMap<>();
private final MultiValueMap<String, T> pathLookup = new LinkedMultiValueMap<>();
diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java
index 900c9edd47..bff5a1cf0a 100644
--- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java
+++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java
@@ -242,6 +242,12 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
*/
@Override
public @Nullable RequestMappingInfo getMatchingCondition(ServerWebExchange exchange) {
+ // Check the path first: when a request has no direct path match, every mapping is checked,
+ // and the path rejects nearly all of them more cheaply than the other conditions.
+ PatternsRequestCondition patterns = this.patternsCondition.getMatchingCondition(exchange);
+ if (patterns == null) {
+ return null;
+ }
RequestMethodsRequestCondition methods = this.methodsCondition.getMatchingCondition(exchange);
if (methods == null) {
return null;
@@ -267,10 +273,6 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
if (version == null) {
return null;
}
- PatternsRequestCondition patterns = this.patternsCondition.getMatchingCondition(exchange);
- if (patterns == null) {
- return null;
- }
RequestConditionHolder custom = this.customConditionHolder.getMatchingCondition(exchange);
if (custom == null) {
return null;
diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java
index 0261368eca..cd42d51664 100644
--- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java
+++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java
@@ -22,7 +22,7 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -542,7 +542,8 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
*/
class MappingRegistry {
- private final Map<T, MappingRegistration<T>> registry = new HashMap<>();
+ // Linked for faster iteration: lookups without a direct path match iterate over all registrations.
+ private final Map<T, MappingRegistration<T>> registry = new LinkedHashMap<>();
private final MultiValueMap<String, T> pathLookup = new LinkedMultiValueMap<>();
diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java
index 80779574da..c30a674f46 100644
--- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java
+++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java
@@ -419,6 +419,22 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
@SuppressWarnings("removal")
@Override
public @Nullable RequestMappingInfo getMatchingCondition(HttpServletRequest request) {
+ // Check the path first: when a request has no direct path match, every mapping is checked,
+ // and the path rejects nearly all of them more cheaply than the other conditions.
+ PathPatternsRequestCondition pathPatterns = null;
+ if (this.pathPatternsCondition != null) {
+ pathPatterns = this.pathPatternsCondition.getMatchingCondition(request);
+ if (pathPatterns == null) {
+ return null;
+ }
+ }
+ PatternsRequestCondition patterns = null;
+ if (this.patternsCondition != null) {
+ patterns = this.patternsCondition.getMatchingCondition(request);
+ if (patterns == null) {
+ return null;
+ }
+ }
RequestMethodsRequestCondition methods = this.methodsCondition.getMatchingCondition(request);
if (methods == null) {
return null;
@@ -443,20 +459,6 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
if (version == null) {
return null;
}
- PathPatternsRequestCondition pathPatterns = null;
- if (this.pathPatternsCondition != null) {
- pathPatterns = this.pathPatternsCondition.getMatchingCondition(request);
- if (pathPatterns == null) {
- return null;
- }
- }
- PatternsRequestCondition patterns = null;
- if (this.patternsCondition != null) {
- patterns = this.patternsCondition.getMatchingCondition(request);
- if (patterns == null) {
- return null;
- }
- }
RequestConditionHolder custom = this.customConditionHolder.getMatchingCondition(request);
if (custom == null) {
return null;
Activity