Skip to content

perf: avoid per-response HttpResponseStatus allocation in statusToNetty#3976

Merged
987Nabil merged 5 commits intomainfrom
hot-path/status-to-netty-allocation
Mar 2, 2026
Merged

perf: avoid per-response HttpResponseStatus allocation in statusToNetty#3976
987Nabil merged 5 commits intomainfrom
hot-path/status-to-netty-allocation

Conversation

@guizmaii
Copy link
Member

@guizmaii guizmaii commented Mar 2, 2026

Summary

  • statusToNetty used the two-arg HttpResponseStatus.valueOf(code, reasonPhrase) which only returns Netty's cached singleton when the reason phrase matches exactly. Since Status.Ok.reasonPhrase produces "Ok" (not "OK"), every 200 OK response allocated a new HttpResponseStatus object (~80-120 bytes).
  • Switch to single-arg valueOf(code) for non-Custom statuses, which always returns the cached instance. Custom statuses with non-empty reason phrases still use the two-arg overload to preserve user-specified phrases.

Test plan

  • Added tests verifying standard status codes return Netty's cached singleton (reference equality)
  • Added test verifying Custom status with non-empty phrase preserves it
  • Added test verifying Custom status with empty phrase falls back to cached instance
  • sbt 'zioHttpJVM/testOnly zio.http.netty.model.ConversionsSpec' passes

The two-arg `HttpResponseStatus.valueOf(code, reasonPhrase)` only returns
the cached Netty singleton when the reason phrase matches exactly.
Since zio-http's `Status.Ok.reasonPhrase` produces "Ok" (not "OK"),
every 200 OK response allocated a new HttpResponseStatus (~80-120 bytes).

Switch to the single-arg `valueOf(code)` for non-Custom statuses,
which always returns the cached instance. Custom statuses with a
non-empty reason phrase still use the two-arg overload.
Copilot AI review requested due to automatic review settings March 2, 2026 06:19
@netlify
Copy link

netlify bot commented Mar 2, 2026

Deploy Preview for zio-http ready!

Name Link
🔨 Latest commit 4c7a0c7
🔍 Latest deploy log https://app.netlify.com/projects/zio-http/deploys/69a54ed7811f130008773463
😎 Deploy Preview https://deploy-preview-3976--zio-http.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@guizmaii guizmaii marked this pull request as draft March 2, 2026 06:20
@guizmaii guizmaii self-assigned this Mar 2, 2026
@guizmaii guizmaii marked this pull request as ready for review March 2, 2026 06:26
@guizmaii guizmaii marked this pull request as draft March 2, 2026 06:33
@guizmaii guizmaii marked this pull request as ready for review March 2, 2026 06:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@guizmaii guizmaii marked this pull request as draft March 2, 2026 06:51
Co-Authored-By: Jules Ivanic <jules.ivanic@gmail.com>
@guizmaii guizmaii marked this pull request as ready for review March 2, 2026 06:55
@guizmaii guizmaii marked this pull request as draft March 2, 2026 08:33
guizmaii added 2 commits March 2, 2026 19:41
Custom status codes now always go through valueOf(code, reasonPhrase)
to preserve the exact reason phrase. Only standard status codes use the
single-arg valueOf(code) for cache hits.

Fixes RoundtripSpec failures where Custom(999, "") was being converted
to Custom(999, "Unknown Status (999)") via Netty's default reason.

Co-Authored-By: Jules Ivanic <jules.ivanic@gmail.com>
Co-Authored-By: Jules Ivanic <jules.ivanic@gmail.com>
@guizmaii guizmaii marked this pull request as ready for review March 2, 2026 08:50
@987Nabil 987Nabil merged commit a365bd6 into main Mar 2, 2026
46 checks passed
@987Nabil 987Nabil deleted the hot-path/status-to-netty-allocation branch March 2, 2026 15:29
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.

3 participants