codeconsole opened a new pull request, #16150:
URL: https://github.com/apache/grails-core/pull/16150
Resolves captured URL mapping names from the match itself rather than from
the dispatching
request, which lets `collectControllerMappings` stop rebuilding the
parameter map for every
candidate mapping on every request.
## Measured
| Benchmark | before ns/op | after ns/op | |
|---|---:|---:|---|
| `ControllerMappingCollectionBenchmark.oneCandidate` | 385.9 | **219.9** |
−43% |
| `ControllerMappingCollectionBenchmark.twoCandidates` | 599.8 | **282.9** |
−53% |
| `ControllerMappingCollectionBenchmark.fourCandidates` | 1241.6 | **777.4**
| −37% |
| `UrlMappingBenchmark.matchCachedHit` | 2.49 | 2.56 | unchanged |
`collectControllerMappings` runs in full on every request even when the URL
mapping cache hits,
so this is on the always-on path. It still costs ~88x the cached match it
wraps; what remains is
`ControllerKey` allocation, the controller map lookup and the sort.
## Why it was slow
A mapping's `controllerName` / `actionName` / `namespace` could be a Closure
that read
`RequestContextHolder.currentRequestAttributes().getParams()` — reaching for
thread-local state to
read a value the mapping already held in its own `params`. To ask a
candidate "which controller are
you?", the framework had to call `webRequest.resetParams()` and
`info.configure(webRequest)` first,
per candidate, cloning the parameter map each time.
Those names now resolve from the match. `configure()` is still called for
mappings whose names are
genuinely request-dependent.
## Deliberately unchanged
`action = { params.goHere }` is a documented feature — those closures are
meant to read the request,
and they still do, taking the old path.
## Behaviour change
A request parameter no longer stands in for a token the URI does not
capture. Under
`"/$controller/$action?"`, `/article?action=gallery` now routes to the
default action rather than
`gallery`; a query string could previously steer which action ran.
`params.action` is still bound
either way. Documented as section 47 of the 8.0 upgrade guide and in the
embedded-variables guide.
This required changing one existing assertion, in
`UrlMappingParameterTests.testNotEqual`, which is
worth a reviewer's attention. That test originally asserted the mapping did
**not** match
(b84ef591f1, GRAILS-2297); when it moved to `UrlMappingsUnitTest` the
fall-through to the default
mapping made `info` non-null and a `controllerName == 'foo'` assertion was
substituted — 'foo' being
a value seeded into the request params by the test itself, i.e. an artifact
of the thread-local
resolution rather than the `notEqual` constraint the test is named for. It
now asserts the value the
URI actually captured.
## Notes
- Adds `grails-web-benchmarks` (opt-in, not part of `build`/`check`). The
same module is added by
#16149; if that lands first this rebases and drops that commit.
- `UrlMappingInfo` gains `isNameResolutionRequestDependent()` as a default
method returning `true`,
so third-party implementations keep current behaviour.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
