feat(routing): memoize RouteMatcher and share one instance with Router#62
Merged
Merged
Conversation
Closes #61. Global middleware that needs to inspect the upcoming route (e.g. PageCacheMiddleware reading `#[Cacheable]`) previously paid for a full route iteration twice per request: once when the middleware called `RouteMatcherInterface::match()`, and again when `Router::handle()` called its own private matcher. Two changes eliminate the duplicate work: 1. **Memoize `RouteMatcher::match()` by (method, path).** The matcher caches both hit and miss results so repeated calls for the same request are O(1). Class is no longer `readonly` since the cache mutates; the `$routes` field remains `readonly`. 2. **Share one RouteMatcher between the container and the Router.** `Router` now takes `RouteMatcherInterface` via constructor instead of constructing a private `RouteMatcher`. `RoutingBootstrapper` creates a single matcher, binds it as `RouteMatcherInterface` in the container, and passes the same instance to `Router`. The `RouteCollection $routes` parameter is removed from `Router` — it only existed to construct the now-removed private matcher. Net effect: a cacheable request still resolves the route exactly once; the middleware's lookup populates the cache and the router's lookup is the cache hit. Tests: - Added 3 RouteMatcher memoization tests (per-key identity, null caching, key disambiguation by method and path). - Added a RoutingBootstrapper test that reflects into the registered Router and asserts its private `$matcher` is the same instance the container hands out for `RouteMatcherInterface`. - Updated RouterTest and DemoRoutingTest constructors from `routes: $routes` to `matcher: new RouteMatcher($routes)`. `composer test` passes (5042 / 0 failed); `phpcs` + `php-cs-fixer` clean on touched files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #61.
Eliminates the duplicate route match per request that any global middleware inspecting the upcoming route would trigger (the original surfacing example was
PageCacheMiddlewarereading#[Cacheable]).RouteMatcher::match()by(method, path)— both hit and miss results are cached, so repeated calls within a request are O(1). Class is no longerreadonlysince the cache mutates; the$routesfield staysreadonly.Router—Routernow takesRouteMatcherInterfacevia constructor instead of constructing a privateRouteMatcher.RoutingBootstrappercreates a single matcher, binds it to the container, and passes the same instance toRouter. TheRouteCollection $routesparameter is removed fromRouter(it only existed to construct the now-removed private matcher).Net effect: a cacheable request still resolves the route exactly once. The middleware's
match()populates the cache; the router's latermatch()is the cache hit.Test plan
RouteMatchermemoization tests: per-(method, path)identity, null caching, and key disambiguation by method and by path.RoutingBootstrappertest that reflects into the registeredRouterand asserts its private$matcheris the same instance the container hands out forRouteMatcherInterface.RouterTest(15 sites) andDemoRoutingTest(2 sites) constructors fromroutes: $routestomatcher: new RouteMatcher($routes).composer test— 5042 passed / 0 failedphpcs+php-cs-fixerclean on touched files🤖 Generated with Claude Code