Skip to content

Commit 0a572be

Browse files
committed
perf(webapp,run-engine): cut CPU on the engine-facing worker-action routes
Three changes from a CPU profile of the worker-action request path, together taking on-CPU time per completed run from 9.07ms to 6.59ms (-27%) at ~300 req/s, with every worker-action p50 down 23-27%. Split the event-loop monitor in two. The blocked-loop detector installs an async_hooks hook that fires for every async resource the process creates, and enabling any async hook also puts V8 on the slow path for promise instrumentation process-wide; it measured ~14% of on-CPU time plus roughly half of all GC. It is now opt-in via EVENT_LOOP_MONITOR_ENABLED (default 0). The event-loop utilization gauge is a single interval timer with no per-request cost, so it moves to EVENT_LOOP_UTILIZATION_MONITOR_ENABLED (default 1) and stays on. Bucket route matching by first static path segment. The existing router patch removed the per-request re-flatten and regex rebuild, but matching was still a linear scan over the whole 521-route table. Route-matching self time drops 64%. Ordering is preserved exactly and equivalence was verified over 20,050 pathnames; apps/webapp/test/routeMatchingPatch.test.ts pins the semantics. Demote the per-heartbeat and per-dequeue info logs to debug. These are the two highest-rate engine calls and each wrote a synchronous structured log line on every request.
1 parent 492d298 commit 0a572be

10 files changed

Lines changed: 291 additions & 29 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: improvement
4+
---
5+
6+
Cut webapp CPU usage by about a quarter on the routes that workers call most, freeing headroom at the same request rate. Detailed event-loop blocking diagnostics are now off by default (set `EVENT_LOOP_MONITOR_ENABLED=1` to restore them); the event-loop utilization metric is unaffected.

apps/webapp/app/entry.server.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type { OperatingSystemPlatform } from "./components/primitives/OperatingS
1818
import { OperatingSystemContextProvider } from "./components/primitives/OperatingSystemProvider";
1919
import { assertRunOpsSplitSentinel, Prisma } from "./db.server";
2020
import { env } from "./env.server";
21-
import { eventLoopMonitor } from "./eventLoopMonitor.server";
21+
import { eventLoopMonitor, eventLoopUtilizationMonitor } from "./eventLoopMonitor.server";
2222
import { logger } from "./services/logger.server";
2323
import { buildImgSrcDirective, parseCspImageOrigins, withImgSrc } from "./utils/cspImageOrigins";
2424
import { singleton } from "./utils/singleton";
@@ -360,6 +360,10 @@ if (env.EVENT_LOOP_MONITOR_ENABLED === "1") {
360360
eventLoopMonitor.enable();
361361
}
362362

363+
if (env.EVENT_LOOP_UTILIZATION_MONITOR_ENABLED === "1") {
364+
eventLoopUtilizationMonitor.enable();
365+
}
366+
363367
if (remoteBuildsEnabled()) {
364368
console.log("🏗️ Remote builds enabled");
365369
} else {

apps/webapp/app/env.server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -995,7 +995,8 @@ const EnvironmentSchema = z
995995

996996
CENTS_PER_RUN: z.coerce.number().default(0),
997997

998-
EVENT_LOOP_MONITOR_ENABLED: z.string().default("1"),
998+
EVENT_LOOP_MONITOR_ENABLED: z.string().default("0"),
999+
EVENT_LOOP_UTILIZATION_MONITOR_ENABLED: z.string().default("1"),
9991000
MAXIMUM_LIVE_RELOADING_EVENTS: z.coerce.number().int().default(1000),
10001001
MAXIMUM_TRACE_SUMMARY_VIEW_COUNT: z.coerce.number().int().default(25_000),
10011002
MAXIMUM_TRACE_DETAILED_SUMMARY_VIEW_COUNT: z.coerce.number().int().default(10_000),

apps/webapp/app/eventLoopMonitor.server.ts

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,25 +89,47 @@ function after(asyncId: number) {
8989
}
9090
}
9191

92+
/**
93+
* Per-async-resource blocked-loop detection. This is the expensive half: the
94+
* hook fires for every async resource the process creates, and enabling any
95+
* async hook also puts V8 on the slow path for promise instrumentation
96+
* process-wide. On a request-heavy instance it costs roughly a seventh of all
97+
* on-CPU time, which is why it is opt-in rather than on by default.
98+
*/
9299
export const eventLoopMonitor = singleton("eventLoopMonitor", () => {
93100
const hook = createHook({ init, before, after, destroy });
94101

95-
let stopEventLoopUtilizationMonitoring: () => void;
96-
97102
return {
98103
enable: () => {
99104
console.log("🥸 Initializing event loop monitor");
100105

101106
hook.enable();
102-
103-
stopEventLoopUtilizationMonitoring = startEventLoopUtilizationMonitoring();
104107
},
105108
disable: () => {
106109
console.log("🥸 Disabling event loop monitor");
107110

108111
hook.disable();
112+
},
113+
};
114+
});
115+
116+
/**
117+
* The cheap half: a single interval timer reading `eventLoopUtilization()`.
118+
* It costs nothing per request, so it stays on by default and is what a
119+
* high-traffic instance should rely on when the async hook is too expensive.
120+
*/
121+
export const eventLoopUtilizationMonitor = singleton("eventLoopUtilizationMonitor", () => {
122+
let stop: (() => void) | undefined;
109123

110-
stopEventLoopUtilizationMonitoring?.();
124+
return {
125+
enable: () => {
126+
console.log("🥸 Initializing event loop utilization monitor");
127+
128+
stop = startEventLoopUtilizationMonitoring();
129+
},
130+
disable: () => {
131+
stop?.();
132+
stop = undefined;
111133
},
112134
};
113135
});
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import { matchRoutes, type RouteObject } from "@remix-run/router";
2+
import { describe, expect, it } from "vitest";
3+
4+
/**
5+
* Guards `patches/@remix-run__router@1.23.3.patch`.
6+
*
7+
* The patch buckets ranked route branches by their first static path segment so
8+
* a request only scans branches that could match it. That is a change to the
9+
* matcher's search order, so these cases pin the behaviour that must survive
10+
* it: branches whose leading segment is dynamic, splat or optional have to stay
11+
* reachable from every pathname, case-insensitive matching has to keep working
12+
* across the bucket lookup, and a more specific static route must still beat a
13+
* dynamic one.
14+
*
15+
* If the patch is ever dropped, these should still pass against the stock
16+
* matcher — they assert matching semantics, not the optimisation.
17+
*/
18+
const routes: RouteObject[] = [
19+
{
20+
id: "root",
21+
path: "/",
22+
children: [
23+
{ id: "index", index: true },
24+
{ id: "engine-dequeue", path: "engine/v1/worker-actions/dequeue" },
25+
{
26+
id: "engine-heartbeat",
27+
path: "engine/v1/worker-actions/runs/:runFriendlyId/snapshots/:snapshotFriendlyId/heartbeat",
28+
},
29+
{ id: "engine-splat", path: "engine/*" },
30+
{ id: "api-run", path: "api/v1/runs/:runId" },
31+
{ id: "api-summary", path: "api/v1/runs/summary" },
32+
{ id: "case-sensitive", path: "Engine/CaseCheck", caseSensitive: true },
33+
{ id: "optional-lang", path: ":lang?/docs" },
34+
{ id: "org-project", path: ":org/projects/:projectId" },
35+
{ id: "orgs-settings", path: "orgs/settings" },
36+
{ id: "orgs-dynamic", path: "orgs/:orgSlug" },
37+
{ id: "catch-all", path: "*" },
38+
],
39+
},
40+
];
41+
42+
function matchedIds(pathname: string, basename?: string): string[] | null {
43+
const matches = matchRoutes(routes, pathname, basename);
44+
return matches ? matches.map((match) => match.route.id!) : null;
45+
}
46+
47+
function paramsFor(pathname: string): Record<string, string | undefined> {
48+
const matches = matchRoutes(routes, pathname);
49+
return matches ? matches[matches.length - 1]!.params : {};
50+
}
51+
52+
describe("route matching (patched matcher)", () => {
53+
it("matches a fully static engine route", () => {
54+
expect(matchedIds("/engine/v1/worker-actions/dequeue")).toEqual(["root", "engine-dequeue"]);
55+
});
56+
57+
it("matches a dynamic engine route and extracts params", () => {
58+
const pathname = "/engine/v1/worker-actions/runs/run_abc/snapshots/snap_def/heartbeat";
59+
expect(matchedIds(pathname)).toEqual(["root", "engine-heartbeat"]);
60+
expect(paramsFor(pathname)).toMatchObject({
61+
runFriendlyId: "run_abc",
62+
snapshotFriendlyId: "snap_def",
63+
});
64+
});
65+
66+
it("prefers a static route over a dynamic sibling at the same depth", () => {
67+
expect(matchedIds("/api/v1/runs/summary")).toEqual(["root", "api-summary"]);
68+
expect(matchedIds("/api/v1/runs/run_abc")).toEqual(["root", "api-run"]);
69+
expect(matchedIds("/orgs/settings")).toEqual(["root", "orgs-settings"]);
70+
expect(matchedIds("/orgs/acme")).toEqual(["root", "orgs-dynamic"]);
71+
});
72+
73+
it("falls back to a splat within the same first segment", () => {
74+
expect(matchedIds("/engine/something/unrouted")).toEqual(["root", "engine-splat"]);
75+
});
76+
77+
it("keeps routes with a dynamic first segment reachable", () => {
78+
expect(matchedIds("/acme/projects/proj_1")).toEqual(["root", "org-project"]);
79+
expect(paramsFor("/acme/projects/proj_1")).toMatchObject({
80+
org: "acme",
81+
projectId: "proj_1",
82+
});
83+
});
84+
85+
it("keeps routes with an optional first segment reachable both ways", () => {
86+
expect(matchedIds("/docs")).toEqual(["root", "optional-lang"]);
87+
expect(matchedIds("/en/docs")).toEqual(["root", "optional-lang"]);
88+
});
89+
90+
it("matches case-insensitively by default", () => {
91+
expect(matchedIds("/ENGINE/v1/worker-actions/dequeue")).toEqual(["root", "engine-dequeue"]);
92+
expect(matchedIds("/API/v1/runs/summary")).toEqual(["root", "api-summary"]);
93+
});
94+
95+
it("honours caseSensitive routes", () => {
96+
expect(matchedIds("/Engine/CaseCheck")).toEqual(["root", "case-sensitive"]);
97+
expect(matchedIds("/engine/casecheck")).toEqual(["root", "engine-splat"]);
98+
});
99+
100+
it("falls through to the global catch-all for an unknown first segment", () => {
101+
expect(matchedIds("/totally/unknown/path")).toEqual(["root", "catch-all"]);
102+
});
103+
104+
it("matches the index route at the root", () => {
105+
expect(matchedIds("/")).toEqual(["root", "index"]);
106+
});
107+
108+
it("still strips a basename before matching", () => {
109+
expect(matchedIds("/base/engine/v1/worker-actions/dequeue", "/base")).toEqual([
110+
"root",
111+
"engine-dequeue",
112+
]);
113+
expect(matchRoutes(routes, "/elsewhere/engine", "/base")).toBeNull();
114+
});
115+
116+
it("matches a percent-encoded first segment", () => {
117+
expect(matchedIds("/%65ngine/v1/worker-actions/dequeue")).toEqual(["root", "engine-dequeue"]);
118+
});
119+
});

internal-packages/run-engine/src/engine/systems/dequeueSystem.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ export class DequeueSystem {
163163
? Math.max(0, Date.now() - message.message.eligibleAtMs)
164164
: undefined;
165165

166-
this.$.logger.info("DequeueSystem.dequeueFromWorkerQueue dequeued message", {
166+
this.$.logger.debug("DequeueSystem.dequeueFromWorkerQueue dequeued message", {
167167
runId,
168168
orgId,
169169
environmentId: message.message.environmentId,

internal-packages/run-engine/src/engine/systems/executionSnapshotSystem.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ export class ExecutionSnapshotSystem {
567567
});
568568
}
569569

570-
this.$.logger.info("heartbeatRun snapshot heartbeat updated", {
570+
this.$.logger.debug("heartbeatRun snapshot heartbeat updated", {
571571
id: latestSnapshot.id,
572572
runId: latestSnapshot.runId,
573573
lastHeartbeatAt: new Date(),

patches/@remix-run__router@1.23.3.patch

Lines changed: 92 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
diff --git a/dist/router.cjs.js b/dist/router.cjs.js
2-
index 6aa7db6fb5a7182afcdf17b16a3356abfa1e7945..95edbde228beff8dbd13fb2800302e31a932ef25 100644
2+
index 6aa7db6fb5a7182afcdf17b16a3356abfa1e7945..5ae0b0e632a356493c3a8b0c88ebd396e8f5305b 100644
33
--- a/dist/router.cjs.js
44
+++ b/dist/router.cjs.js
5-
@@ -783,6 +783,11 @@ function convertRoutesToDataRoutes(routes, mapRouteProperties, parentPath, manif
5+
@@ -783,6 +783,51 @@ function convertRoutesToDataRoutes(routes, mapRouteProperties, parentPath, manif
66
*
77
* @see https://reactrouter.com/v6/utils/match-routes
88
*/
@@ -11,10 +11,50 @@ index 6aa7db6fb5a7182afcdf17b16a3356abfa1e7945..95edbde228beff8dbd13fb2800302e31
1111
+// fix #14967; maintainer suggested patch-package until the Remix 3 route-pattern rewrite).
1212
+let __branchCache = new WeakMap();
1313
+let __compileCache = new Map();
14+
+/**
15+
+ * trigger.dev perf patch 2 — bucket ranked branches by their first static path
16+
+ * segment so a request scans only branches that could match it, rather than the
17+
+ * whole 500+ route table. See patches/README.md.
18+
+ */
19+
+let __bucketCache = new WeakMap();
20+
+/**
21+
+ * Returns the lowercased leading segment when it is static, or null when the
22+
+ * branch can match any first segment (dynamic, splat or optional leading
23+
+ * segment, or a root/pathless path) and so must always be considered.
24+
+ */
25+
+function __firstStaticSegment(path) {
26+
+ if (!path || path === "/") return null;
27+
+ let start = path.charCodeAt(0) === 47 ? 1 : 0;
28+
+ let end = path.indexOf("/", start);
29+
+ let seg = end === -1 ? path.slice(start) : path.slice(start, end);
30+
+ if (seg === "") return null;
31+
+ if (seg.indexOf(":") !== -1 || seg.indexOf("*") !== -1 || seg.indexOf("(") !== -1 || seg.indexOf("?") !== -1) {
32+
+ return null;
33+
+ }
34+
+ return seg.toLowerCase();
35+
+}
36+
+function __buildBuckets(branches) {
37+
+ let byFirstSegment = new Map();
38+
+ let always = [];
39+
+ for (let i = 0; i < branches.length; ++i) {
40+
+ let seg = __firstStaticSegment(branches[i].path);
41+
+ if (seg === null) {
42+
+ always.push(i);
43+
+ continue;
44+
+ }
45+
+ let list = byFirstSegment.get(seg);
46+
+ if (!list) {
47+
+ list = [];
48+
+ byFirstSegment.set(seg, list);
49+
+ }
50+
+ list.push(i);
51+
+ }
52+
+ return { byFirstSegment, always };
53+
+}
1454
function matchRoutes(routes, locationArg, basename) {
1555
if (basename === void 0) {
1656
basename = "/";
17-
@@ -795,8 +800,13 @@ function matchRoutesImpl(routes, locationArg, basename, allowPartial) {
57+
@@ -795,18 +840,51 @@ function matchRoutesImpl(routes, locationArg, basename, allowPartial) {
1858
if (pathname == null) {
1959
return null;
2060
}
@@ -29,8 +69,54 @@ index 6aa7db6fb5a7182afcdf17b16a3356abfa1e7945..95edbde228beff8dbd13fb2800302e31
2969
+ }
3070
let matches = null;
3171
let decoded = decodePath(pathname);
32-
for (let i = 0; matches == null && i < branches.length; ++i) {
33-
@@ -1115,6 +1125,12 @@ function compilePath(path, caseSensitive, end) {
72+
- for (let i = 0; matches == null && i < branches.length; ++i) {
73+
- // Incoming pathnames are generally encoded from either window.location
74+
- // or from router.navigate, but we want to match against the unencoded
75+
- // paths in the route definitions. Memory router locations won't be
76+
- // encoded here but there also shouldn't be anything to decode so this
77+
- // should be a safe operation. This avoids needing matchRoutes to be
78+
- // history-aware.
79+
- matches = matchRouteBranch(branches[i], decoded, allowPartial);
80+
+ // Incoming pathnames are generally encoded from either window.location
81+
+ // or from router.navigate, but we want to match against the unencoded
82+
+ // paths in the route definitions. Memory router locations won't be
83+
+ // encoded here but there also shouldn't be anything to decode so this
84+
+ // should be a safe operation. This avoids needing matchRoutes to be
85+
+ // history-aware.
86+
+ let buckets = __bucketCache.get(branches);
87+
+ if (!buckets) {
88+
+ buckets = __buildBuckets(branches);
89+
+ __bucketCache.set(branches, buckets);
90+
+ }
91+
+ let requestSegment = __firstStaticSegment(decoded);
92+
+ if (requestSegment === null) {
93+
+ for (let i = 0; matches == null && i < branches.length; ++i) {
94+
+ matches = matchRouteBranch(branches[i], decoded, allowPartial);
95+
+ }
96+
+ return matches;
97+
+ }
98+
+ /**
99+
+ * Both lists hold indexes into the already rank-sorted `branches`, so walking
100+
+ * them in ascending-index order preserves the exact evaluation order the
101+
+ * unbucketed scan would have used.
102+
+ */
103+
+ let scoped = buckets.byFirstSegment.get(requestSegment);
104+
+ let always = buckets.always;
105+
+ let si = 0;
106+
+ let ai = 0;
107+
+ let scopedLength = scoped === undefined ? 0 : scoped.length;
108+
+ while (matches == null && (si < scopedLength || ai < always.length)) {
109+
+ let index;
110+
+ if (si < scopedLength && (ai >= always.length || scoped[si] < always[ai])) {
111+
+ index = scoped[si++];
112+
+ } else {
113+
+ index = always[ai++];
114+
+ }
115+
+ matches = matchRouteBranch(branches[index], decoded, allowPartial);
116+
}
117+
return matches;
118+
}
119+
@@ -1115,6 +1193,12 @@ function compilePath(path, caseSensitive, end) {
34120
if (end === void 0) {
35121
end = true;
36122
}
@@ -43,7 +129,7 @@ index 6aa7db6fb5a7182afcdf17b16a3356abfa1e7945..95edbde228beff8dbd13fb2800302e31
43129
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(/\*$/, "/*") + "\"."));
44130
let params = [];
45131
let regexpSource = "^" + path.replace(/\/*\*?$/, "") // Ignore trailing / and /*, we'll handle it below
46-
@@ -1147,7 +1163,11 @@ function compilePath(path, caseSensitive, end) {
132+
@@ -1147,7 +1231,11 @@ function compilePath(path, caseSensitive, end) {
47133
regexpSource += "(?:(?=\\/|$))";
48134
} else ;
49135
let matcher = new RegExp(regexpSource, caseSensitive ? undefined : "i");

0 commit comments

Comments
 (0)