ReviewOS

stacks/bun-router

perf(middleware): preserve synchronous chains

#905
Closed chris opened this 8 days ago ·
8 days ago

Problem

buildMiddlewareChain() wraps every middleware layer and every next() call in an async function. A completely synchronous middleware chain therefore creates and awaits a promise at each layer, including Stacks' normal maintenance pass-through path.

Proposed change

Compose middleware with plain functions. Return synchronous Response values directly, and attach a continuation only when a downstream layer returns a promise. Preserve null-to-200 fallback behavior, synchronous throws, rejected promises, middleware ordering, short circuits, and repeated next() behavior.

Acceptance criteria

  • Keep synchronous middleware chains synchronous.
  • Preserve asynchronous middleware and error behavior.
  • Add deterministic coverage for sync chaining, async chaining, short circuits, and thrown errors.
  • Improve representative middleware and downstream Stacks request benchmarks on exact Bun 1.4.2.
  • Pass the complete suite, typechecks, lint, and package build.
  • Keep stable package publication pending explicit release approval.
8 days ago

Implemented and pushed to main in two small commits:

  • 2c15d2e: preserve synchronous middleware chains while keeping the public builder Promise-compatible
  • a22ca57: record the change in the 0.1.19 changelog

The internal compiler now returns synchronous responses directly, creates continuations only for asynchronous results, and is used by the stock run and cached dispatch paths. Public buildMiddlewareChain() callers still receive a Promise. Builder overrides, middleware ordering, short circuits, null-to-200 fallback behavior, rejected errors, repeated next() calls, and native thenable assimilation remain compatible.

All measurements used exact Bun 1.4.2. The benchmark compared 78df433 against the final implementation in 16 alternating process pairs.

bun-router middleware results

WorkloadBaselineCandidateThroughputCandidate winsPaired elapsed 95% CI
Public sync chain, 250k ops5,915,136 ops/s20,943,583 ops/s+254.07%16/16-30.56ms to -30.10ms
Public async chain, 250k ops3,823,085 ops/s6,318,021 ops/s+65.26%16/16-26.68ms to -24.96ms
Cached sync route dispatch, 50k ops3,709,101 ops/s6,629,621 ops/s+78.74%16/16-6.11ms to -5.77ms
Cached async route dispatch, 50k ops2,559,824 ops/s3,628,938 ops/s+41.77%16/16-6.04ms to -5.47ms

Artifact SHA-256: 5c96f4ba402a015f753758daad5011ad0c49ae6591403c649fe542759bce31f9

Downstream Stacks request result

The warmed production serverResponse() missing-route path includes Stacks' default maintenance pass-through middleware and the enriched 404 path from #904.

  • 351,179 to 359,854 requests/s
  • +2.47% throughput, -2.41% elapsed
  • 16/16 candidate wins
  • paired elapsed 95% CI: -4.20ms to -2.66ms per 50,000 requests
  • artifact SHA-256: d73443610a3b511916947ee243948b65681d1d599f3f9a97652b8f84807e0cba

Validation

  • 1,026 tests passed across 63 files, 0 failed, 3,103 assertions
  • standard and registry TypeScript projects passed
  • pickier lint passed
  • package build and compiled CLI passed
  • final review found no remaining Standards or Spec issues

This is ready for the pending 0.1.19 release tracked in #899. Stable publication remains pending explicit release approval. Downstream adoption remains tracked in stacksjs/stacks#2727.

Sign in to comment on this issue.