From d841ca2842a7fbb04ddab4b741ef006cc894a6a2 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Sat, 10 Oct 2026 18:35:22 -0400 Subject: [PATCH] perf(http): skip route lifecycle work when nothing listens The per-route lifecycle middleware built a payload, opened an event.emitAndWait span and hooked finish/close on every request, listeners or not. Each phase now checks hasListeners first. EventClient's three copies of the key-prefix walk become one memoized #matchKeys. --- src/backend/clients/event/EventClient.test.ts | 24 ++++++ src/backend/clients/event/EventClient.ts | 72 +++++++++-------- src/backend/core/http/routeLifecycle.test.ts | 79 +++++++++++++++++-- src/backend/core/http/routeLifecycle.ts | 21 ++++- 4 files changed, 157 insertions(+), 39 deletions(-) diff --git a/src/backend/clients/event/EventClient.test.ts b/src/backend/clients/event/EventClient.test.ts index 4e09043ab..81f6d619a 100644 --- a/src/backend/clients/event/EventClient.test.ts +++ b/src/backend/clients/event/EventClient.test.ts @@ -136,6 +136,30 @@ describe('EventClient', () => { }); }); + describe('repeat emits', () => { + it('reaches listeners added after the key was first emitted', () => { + target.emit(`${key}.a.b`, {}, {}); + const wildcard = vi.fn(); + const exact = vi.fn(); + target.on(`${key}.a.*`, wildcard); + target.on(`${key}.a.b`, exact); + target.emit(`${key}.a.b`, {}, {}); + expect(wildcard).toHaveBeenCalledTimes(1); + expect(exact).toHaveBeenCalledTimes(1); + expect(target.hasListeners(`${key}.a.b`)).toBe(true); + }); + + it('does not skip a listener when an earlier one unsubscribes mid-dispatch', () => { + const second = vi.fn(); + const first = vi.fn(() => target.off(key, first)); + target.on(key, first); + target.on(key, second); + target.emit(key, {}, {}); + expect(first).toHaveBeenCalledTimes(1); + expect(second).toHaveBeenCalledTimes(1); + }); + }); + describe('emitAndWait', () => { it('awaits async listeners before resolving', async () => { let resolved = false; diff --git a/src/backend/clients/event/EventClient.ts b/src/backend/clients/event/EventClient.ts index a3591279d..1f71d5658 100644 --- a/src/backend/clients/event/EventClient.ts +++ b/src/backend/clients/event/EventClient.ts @@ -18,6 +18,7 @@ */ import { extensionStore } from '../../extensions'; +import { BoundedTtlMap } from '../../util/boundedTtlMap.js'; import { withSpan } from '../../util/span.js'; import { PuterClient } from '../types'; import { @@ -28,8 +29,13 @@ import { MatchingEvents, } from './types'; +const NO_LISTENERS: readonly EventListener[] = []; + export class EventClient extends PuterClient { #eventListeners: Partial> = {}; + #matchKeyMemo = new BoundedTtlMap({ + maxEntries: 1024, + }); onServerStart() { this.emit('serverStart', {}, {}); @@ -63,19 +69,8 @@ export class EventClient extends PuterClient { data: EventMap[T], meta: EventMetadata, ) { - const parts = key.split('.'); - for (let i = 0; i < parts.length; i++) { - const matchKey = ( - i === parts.length - 1 - ? key - : `${parts.slice(0, i + 1).join('.')}.*` - ) as ListenKey; - const extensionListeners = extensionStore.events[matchKey]; - const listeners = (this.#eventListeners[matchKey] || []).concat( - extensionListeners || [], - ); - if (!listeners) continue; - for (const listener of listeners) { + for (const matchKey of this.#matchKeys(key)) { + for (const listener of this.#listenersFor(matchKey)) { this.#emitEvent(listener, key, data, meta); } } @@ -102,21 +97,10 @@ export class EventClient extends PuterClient { // Spanned because callers block on listeners — this is where time // spent in extension hooks (e.g. `ip.validate`) gets attributed. return withSpan('event.emitAndWait', { 'event.key': key }, async () => { - const parts = key.split('.'); - for (let i = 0; i < parts.length; i++) { - const matchKey = ( - i === parts.length - 1 - ? key - : `${parts.slice(0, i + 1).join('.')}.*` - ) as ListenKey; - const extensionListeners = extensionStore.events[matchKey]; - const listeners = (this.#eventListeners[matchKey] || []).concat( - extensionListeners || [], - ); - if (!listeners) continue; + for (const matchKey of this.#matchKeys(key)) { // A wildcard listener observes the event; it does not implement it. const isExact = matchKey === key; - for (const listener of listeners) { + for (const listener of this.#listenersFor(matchKey)) { try { await listener(key, data, meta); } catch (e) { @@ -142,13 +126,7 @@ export class EventClient extends PuterClient { * otherwise perfectly cheap and doesn't need this. */ hasListeners(key: T): boolean { - const parts = key.split('.'); - for (let i = 0; i < parts.length; i++) { - const matchKey = ( - i === parts.length - 1 - ? key - : `${parts.slice(0, i + 1).join('.')}.*` - ) as ListenKey; + for (const matchKey of this.#matchKeys(key)) { if (this.#eventListeners[matchKey]?.length) return true; if (extensionStore.events[matchKey]?.length) return true; } @@ -191,6 +169,34 @@ export class EventClient extends PuterClient { const idx = listeners.indexOf(callback as EventListener); if (idx !== -1) listeners.splice(idx, 1); } + /** + * The keys a listener can be registered under to hear `key`: each + * `.*` shorter than the key, then the key itself. Memoized, since + * the same few keys are emitted on every request. + */ + #matchKeys(key: string): readonly ListenKey[] { + const cached = this.#matchKeyMemo.get(key); + if (cached) return cached; + const parts = key.split('.'); + const matchKeys = parts.map( + (_, i) => + (i === parts.length - 1 + ? key + : `${parts.slice(0, i + 1).join('.')}.*`) as ListenKey, + ); + this.#matchKeyMemo.set(key, matchKeys); + return matchKeys; + } + + #listenersFor(matchKey: ListenKey): readonly EventListener[] { + const own = this.#eventListeners[matchKey]; + const fromExtensions = extensionStore.events[matchKey]; + if (!own?.length && !fromExtensions?.length) return NO_LISTENERS; + // A copy, so a listener that unsubscribes mid-dispatch can't make the + // loop skip its neighbour. + return (own ?? []).concat(fromExtensions ?? []); + } + async #emitEvent( listener: EventListener, key: T, diff --git a/src/backend/core/http/routeLifecycle.test.ts b/src/backend/core/http/routeLifecycle.test.ts index 31cb8c94e..4326bcd21 100644 --- a/src/backend/core/http/routeLifecycle.test.ts +++ b/src/backend/core/http/routeLifecycle.test.ts @@ -19,12 +19,9 @@ import { EventEmitter } from 'node:events'; import type { Request, Response } from 'express'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { EventClient } from '../../clients/event/EventClient'; -import type { - EventMap, - RouteLifecycleEvent, -} from '../../clients/event/types'; +import type { EventMap, RouteLifecycleEvent } from '../../clients/event/types'; import type { IConfig } from '../../types'; import { createRouteLifecycleMiddleware, @@ -98,6 +95,78 @@ describe('routeEventKeyBase', () => { }); describe('createRouteLifecycleMiddleware', () => { + it('does no event work for a route nobody listens to', async () => { + const events = makeEvents(); + // A listener elsewhere must not make this route pay. + record(events, 'route.get.other.before' as keyof EventMap); + const emitAndWait = vi.spyOn(events, 'emitAndWait'); + const emit = vi.spyOn(events, 'emit'); + + const mw = createRouteLifecycleMiddleware(events, POST, PATH); + const res = new FakeRes(); + let nextCalled = false; + await mw(makeReq('u-1'), res as unknown as Response, () => { + nextCalled = true; + }); + res.writableFinished = true; + res.emit('finish'); + res.emit('close'); + + expect(nextCalled).toBe(true); + expect(emitAndWait).not.toHaveBeenCalled(); + expect(emit).not.toHaveBeenCalled(); + expect(res.listenerCount('finish')).toBe(0); + expect(res.listenerCount('close')).toBe(0); + }); + + it('skips the awaited before emit when only the terminal phase is observed', async () => { + const events = makeEvents(); + const after = record(events, `${BASE}.after` as keyof EventMap); + const emitAndWait = vi.spyOn(events, 'emitAndWait'); + + const mw = createRouteLifecycleMiddleware(events, POST, PATH); + const res = new FakeRes(); + await mw(makeReq('u-1'), res as unknown as Response, () => {}); + res.writableFinished = true; + res.emit('finish'); + + expect(emitAndWait).not.toHaveBeenCalled(); + expect(after).toHaveLength(1); + expect(after[0]).toMatchObject({ + phase: 'after', + actorUid: 'user:u-1', + }); + }); + + it('skips the response hooks when only before is observed', async () => { + const events = makeEvents(); + const before = record(events, `${BASE}.before` as keyof EventMap); + + const mw = createRouteLifecycleMiddleware(events, POST, PATH); + const res = new FakeRes(); + let nextCalled = false; + await mw(makeReq(), res as unknown as Response, () => { + nextCalled = true; + }); + + expect(nextCalled).toBe(true); + expect(before).toHaveLength(1); + expect(res.listenerCount('finish')).toBe(0); + }); + + it('fires wildcard listeners on every phase', async () => { + const events = makeEvents(); + const seen = record(events, 'route.*' as keyof EventMap); + + const mw = createRouteLifecycleMiddleware(events, POST, PATH); + const res = new FakeRes(); + await mw(makeReq(), res as unknown as Response, () => {}); + res.writableFinished = true; + res.emit('finish'); + + expect(seen.map((e) => e.phase)).toEqual(['before', 'after']); + }); + it('emits before, then after on a clean finish', async () => { const events = makeEvents(); const before = record(events, `${BASE}.before` as keyof EventMap); diff --git a/src/backend/core/http/routeLifecycle.ts b/src/backend/core/http/routeLifecycle.ts index 6a9e0548e..1b4ecdb7c 100644 --- a/src/backend/core/http/routeLifecycle.ts +++ b/src/backend/core/http/routeLifecycle.ts @@ -61,6 +61,8 @@ export const routeEventKeyBase = ( * off the real status, not `reject`. * * Subscribers can listen on `route.*`, `route..*`, or the exact key. + * Each phase is skipped when nothing listens for it, so an unobserved route + * pays for a few map lookups and nothing else. */ export const createRouteLifecycleMiddleware = ( events: EventClient, @@ -70,8 +72,18 @@ export const createRouteLifecycleMiddleware = ( const pathLabel = typeof fullPath === 'string' ? fullPath : String(fullPath); const keyBase = routeEventKeyBase(method, fullPath); + const beforeKey = `${keyBase}.before` as const; return async (req: Request, res, next) => { + const observeBefore = events.hasListeners(beforeKey); + const observeEnd = + events.hasListeners(`${keyBase}.after`) || + events.hasListeners(`${keyBase}.error`); + if (!observeBefore && !observeEnd) { + next(); + return; + } + const actor = req.actor ? actorUid(req.actor) : undefined; const startedAt = Date.now(); const base = { @@ -89,7 +101,9 @@ export const createRouteLifecycleMiddleware = ( allow: true as boolean, rejectReason: undefined as string | undefined, }; - await events.emitAndWait(`${keyBase}.before`, beforeEvent, {}); + if (observeBefore) { + await events.emitAndWait(beforeKey, beforeEvent, {}); + } // Explicit veto: a listener set `allow = false`. Emit `reject` and // answer 403 — unless the listener already wrote its own response, in @@ -137,6 +151,11 @@ export const createRouteLifecycleMiddleware = ( return; } + if (!observeEnd) { + next(); + return; + } + let settled = false; const settle = (aborted: boolean) => { if (settled) return;