Skip to content

chore: remove unreliable octet stream benchmarks - #2188

Merged
dinwwwh merged 2 commits into
mainfrom
claude/octet-stream-bench-reliability-87d858
Oct 6, 2026
Merged

dinwwwh merged 2 commits into
mainfrom
claude/octet-stream-bench-reliability-87d858

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Oct 6, 2026

Copy link
Copy Markdown
Member

Removes the `octet stream` benchmarks from the rpc and openapi link + handler suites. They kept producing false CodSpeed regressions and improvements on PRs that don't touch streaming, so CodSpeed reports become trustworthy again.

Why

Testing

  • `buffered` and `event stream` benches in both files still run.
  • Lint and type-check are clean for the touched files.

The rpc and openapi link + handler octet stream benches flip between two
values (~640 µs vs ~728 µs for rpc) across CI runs with no code changes,
producing false ±12-14% CodSpeed regressions on unrelated PRs. Remove them
and the payload helpers only they used.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
orpc 93b943e Commit Preview URL

Branch Preview URL
Oct 06 2026, 06:19 AM

@pkg-pr-new

pkg-pr-new Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
More templates

@orpc/ai-sdk

npm i https://pkg.pr.new/@orpc/ai-sdk@2188

@orpc/arktype

npm i https://pkg.pr.new/@orpc/arktype@2188

@orpc/bun

npm i https://pkg.pr.new/@orpc/bun@2188

@orpc/client

npm i https://pkg.pr.new/@orpc/client@2188

@orpc/cloudflare

npm i https://pkg.pr.new/@orpc/cloudflare@2188

@orpc/contract

npm i https://pkg.pr.new/@orpc/contract@2188

@orpc/experimental-effect

npm i https://pkg.pr.new/@orpc/experimental-effect@2188

@orpc/evlog

npm i https://pkg.pr.new/@orpc/evlog@2188

@orpc/hibernation

npm i https://pkg.pr.new/@orpc/hibernation@2188

@orpc/json-schema

npm i https://pkg.pr.new/@orpc/json-schema@2188

@orpc/experimental-lock

npm i https://pkg.pr.new/@orpc/experimental-lock@2188

@orpc/experimental-msw

npm i https://pkg.pr.new/@orpc/experimental-msw@2188

@orpc/nest

npm i https://pkg.pr.new/@orpc/nest@2188

@orpc/next

npm i https://pkg.pr.new/@orpc/next@2188

@orpc/node

npm i https://pkg.pr.new/@orpc/node@2188

@orpc/openapi

npm i https://pkg.pr.new/@orpc/openapi@2188

@orpc/opentelemetry

npm i https://pkg.pr.new/@orpc/opentelemetry@2188

@orpc/pinia-colada

npm i https://pkg.pr.new/@orpc/pinia-colada@2188

@orpc/pino

npm i https://pkg.pr.new/@orpc/pino@2188

@orpc/publisher

npm i https://pkg.pr.new/@orpc/publisher@2188

@orpc/ratelimit

npm i https://pkg.pr.new/@orpc/ratelimit@2188

@orpc/server

npm i https://pkg.pr.new/@orpc/server@2188

@orpc/shared

npm i https://pkg.pr.new/@orpc/shared@2188

@orpc/swr

npm i https://pkg.pr.new/@orpc/swr@2188

@orpc/tanstack-query

npm i https://pkg.pr.new/@orpc/tanstack-query@2188

@orpc/trpc

npm i https://pkg.pr.new/@orpc/trpc@2188

@orpc/valibot

npm i https://pkg.pr.new/@orpc/valibot@2188

@orpc/zod

npm i https://pkg.pr.new/@orpc/zod@2188

commit: 93b943e

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Dropping the ReadableStream branch from the shared drainBody helper also changes the static-file benchmarks, which is outside this PR's stated scope and should be addressed before merge.

Reviewed changes

  • Removed the two octet stream benches — deleted bench('octet stream', ...) from benches/rpc-link-handler.bench.ts and benches/openapi-link-handler.bench.ts plus their now-unused imports; buffered and event stream remain.
  • Pruned shared payload helpers — removed asReadableStream, splitBytes, BYTES_1KB/10KB/100KB/5MB, and the unused SIZE_1KB/10KB/100KB; SIZE_5MB is retained for FILE_BYTES.
  • Reduced drainBody — kept only the isAsyncIteratorObject branch; the ReadableStream draining path was removed along with the octet benches.

No dangling references to the removed helpers remain in benches/.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment thread benches/__shared__/payloads.ts
@codecov

codecov Bot commented Oct 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will improve performance by 21.35%

⚡ 1 improved benchmark
✅ 27 untouched benchmarks
⏩ 2 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ 1KB payload 305.5 µs 251.7 µs +21.35%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/octet-stream-bench-reliability-87d858 (93b943e) with main (ee02653)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

static-file-handler benches still pass ReadableStream response bodies to
drainBody; without this branch they stopped reading the file and
under-measured the serve path.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

The delta since the last review (2456e6a) restores the ReadableStream draining branch in drainBody, which resolves the static-file regression flagged previously. The removal cleanup itself is clean and free of dangling references.

Reviewed changes

  • Restored the ReadableStream drain path in drainBody — benches/__shared__/payloads.ts:85-98 again consumes ReadableStream response bodies, so benches/static-file-handler.bench.ts once more measures the full serve path and releases the reader. This was the sole concern from the prior review and is now addressed.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@dinwwwh
dinwwwh merged commit acbef83 into main Oct 6, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant