Eric Allam
3bc88c453e
## Summary Under high request load the webapp spends most of its CPU inside react-router's `matchRoutes`, not in application code. `@remix-run/router@1.23.2` (the React Router v6 / Remix 2 core) re-flattens, re-ranks, and recompiles the entire route table on every request, and with the webapp's ~436 routes that cost dominates once request rates climb. There is no `NODE_ENV` gate, so production pays it too. This adds a pnpm patch that memoizes the parts that depend only on the static route manifest: it caches the flattened/ranked branches per route tree, hoists the loop-invariant `decodePath` out of the match loop, and caches compiled path regexes. ## Benchmark CPU profile over the same load (100 concurrent tag feeds, ~425 req/s), `NODE_ENV=production`, before vs after the patch: | Metric | Before | After | | --- | --- | --- | | Active CPU (self-time over the window) | 28.3s | 18.5s (-34%) | | Route-matching self-time | 19.2s | 7.5s (-61%) | | Event-loop lag p99 | 322ms | 113ms (-65%) | | Idle headroom | 26% | 52% | Application/realtime code was ~0% of CPU in both profiles; the bottleneck was entirely generic per-request route matching. ## Why a patch instead of an upgrade The inefficiency is acknowledged upstream ([remix-run/react-router#8653](https://github.com/remix-run/react-router/issues/8653)). A contributor PR doing exactly this ([remix-run/react-router#14866](https://github.com/remix-run/react-router/pull/14866)) was closed in favor of a narrower fix ([remix-run/react-router#14967](https://github.com/remix-run/react-router/pull/14967), branch caching only, shipped in React Router v7), with the maintainer suggesting patch-package as the interim until the Remix 3 route-pattern rewrite (see [remix-run/remix#4786](https://github.com/remix-run/remix/discussions/4786)). We are on the v6-era core and cannot pick up even the partial fix without a framework migration, so this patch is the sanctioned stopgap, and it also includes the compiled-regex cache the merged PR left out. [`patches/README.md`](https://github.com/triggerdotdev/trigger.dev/blob/perf/react-router-route-matching/patches/README.md) documents the full rationale, the safety argument (deterministic, internal-only, bounded caches), and when to remove the patch.
70 lines
3.3 KiB
Diff
70 lines
3.3 KiB
Diff
diff --git a/dist/router.cjs.js b/dist/router.cjs.js
|
|
index e634d45fee327b5f9ef63eee8dc1da39b07c79d4..ce1cf6c599e7efa82d51b63d26c0c92e82931083 100644
|
|
--- a/dist/router.cjs.js
|
|
+++ b/dist/router.cjs.js
|
|
@@ -746,6 +746,11 @@ function convertRoutesToDataRoutes(routes, mapRouteProperties, parentPath, manif
|
|
*
|
|
* @see https://reactrouter.com/v6/utils/match-routes
|
|
*/
|
|
+// trigger.dev perf patch — memoize per-request route matching. See patches/README.md
|
|
+// (backports the idea in react-router PR #14866, which was closed in favor of the partial
|
|
+// fix #14967; maintainer suggested patch-package until the Remix 3 route-pattern rewrite).
|
|
+let __branchCache = new WeakMap();
|
|
+let __compileCache = new Map();
|
|
function matchRoutes(routes, locationArg, basename) {
|
|
if (basename === void 0) {
|
|
basename = "/";
|
|
@@ -758,17 +763,17 @@ function matchRoutesImpl(routes, locationArg, basename, allowPartial) {
|
|
if (pathname == null) {
|
|
return null;
|
|
}
|
|
- let branches = flattenRoutes(routes);
|
|
- rankRouteBranches(branches);
|
|
+ // flatten+rank depend only on `routes` (static) — cache per route-tree ref.
|
|
+ let branches = __branchCache.get(routes);
|
|
+ if (!branches) {
|
|
+ branches = flattenRoutes(routes);
|
|
+ rankRouteBranches(branches);
|
|
+ __branchCache.set(routes, branches);
|
|
+ }
|
|
let matches = null;
|
|
+ // decodePath(pathname) is loop-invariant — hoisted out (was recomputed per branch).
|
|
+ let decoded = decodePath(pathname);
|
|
for (let i = 0; matches == null && i < branches.length; ++i) {
|
|
- // Incoming pathnames are generally encoded from either window.location
|
|
- // or from router.navigate, but we want to match against the unencoded
|
|
- // paths in the route definitions. Memory router locations won't be
|
|
- // encoded here but there also shouldn't be anything to decode so this
|
|
- // should be a safe operation. This avoids needing matchRoutes to be
|
|
- // history-aware.
|
|
- let decoded = decodePath(pathname);
|
|
matches = matchRouteBranch(branches[i], decoded, allowPartial);
|
|
}
|
|
return matches;
|
|
@@ -1078,6 +1083,12 @@ function compilePath(path, caseSensitive, end) {
|
|
if (end === void 0) {
|
|
end = true;
|
|
}
|
|
+ // perf patch: cache the compiled [regexp, params] by pattern (see patches/README.md).
|
|
+ let __ck = path + "\0" + caseSensitive + "\0" + end;
|
|
+ let __cc = __compileCache.get(__ck);
|
|
+ if (__cc !== void 0) {
|
|
+ return __cc;
|
|
+ }
|
|
warning(path === "*" || !path.endsWith("*") || path.endsWith("/*"), "Route path \"" + path + "\" will be treated as if it were " + ("\"" + path.replace(/\*$/, "/*") + "\" because the `*` character must ") + "always follow a `/` in the pattern. To get rid of this warning, " + ("please change the route path to \"" + path.replace(/\*$/, "/*") + "\"."));
|
|
let params = [];
|
|
let regexpSource = "^" + path.replace(/\/*\*?$/, "") // Ignore trailing / and /*, we'll handle it below
|
|
@@ -1110,7 +1121,11 @@ function compilePath(path, caseSensitive, end) {
|
|
regexpSource += "(?:(?=\\/|$))";
|
|
} else ;
|
|
let matcher = new RegExp(regexpSource, caseSensitive ? undefined : "i");
|
|
- return [matcher, params];
|
|
+ let __res = [matcher, params];
|
|
+ // Bounded: route patterns are a static set; the cap guards any dynamic matchPath() use.
|
|
+ if (__compileCache.size >= 2000) __compileCache.clear();
|
|
+ __compileCache.set(__ck, __res);
|
|
+ return __res;
|
|
}
|
|
function decodePath(value) {
|
|
try {
|