warp: compose response header directly into the write buffer - #1094
Open
seanparsons wants to merge 2 commits into
Open
warp: compose response header directly into the write buffer#1094seanparsons wants to merge 2 commits into
seanparsons wants to merge 2 commits into
Conversation
For RspBuilder responses (responseBuilder/responseLBS, the most common
response type) the header block was copied twice: composeHeader
allocated a fresh pinned ByteString and filled it, then the
byteString builder copied those bytes again into the connection's
write buffer (response headers are nearly always < 4096 bytes, so
bytestring's builder copies rather than inserts).
Changes:
* ResponseHeader gains composeHeaderPtr (write the status line and
headers at a raw pointer, returning the length) and
composeHeaderLength; composeHeader is now a thin wrapper, sharing
the copy loops.
* IO gains toBufIOWithOffset, the existing loop generalized with a
starting offset; the seed bytes are flushed with the first batch
and counted in the returned total, so the length reported to the
logger is unchanged. toBufIOWith = toBufIOWithOffset 0.
* sendRsp composes the header in place when it fits in the current
write buffer and runs the (possibly chunked) body builder after
it; oversized headers fall back to the old path.
Wire bytes verified byte-identical against master for content-length,
chunked, and 20-header responses (only Date differing). Measured:
sendResponse 4 headers -6%, 20 headers -11%, about -430 bytes
allocated per request.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011HBrMGfWRxTpYeEB4UT8Kf
warp: stop rebuilding clean response headers, index in one pass
sendResponse traversed and rebuilt the response header list 5-6 times
per response:
* sanitizeHeaders ran map (sanitize <$>) unconditionally, allocating
a fresh list + tuples per response even though header values almost
never contain CR/LF, and scanned every value byte with S.any.
* indexResponseHeader built a boxed Data.Array via runSTArray even
though only four slots (Content-Length, Server, Date,
Last-Modified) are ever consulted.
Changes:
* sanitizeHeaders now detects dirty values first (S.elemIndex, i.e.
memchr, for CR and LF) and returns the input list untouched in the
clean case; the rebuild happens only when something actually needs
sanitizing. Dirty-value output is unchanged.
* IndexedResponseHeader is a four-field strict record filled in a
single list traversal; call sites use plain selectors instead of
bounds-checked array reads. The dead ResponseHeaderIndex enum is
removed.
Measured (GHC 9.10.3, warp:bench:response): sendResponse with 4
headers 1.58us -> 1.39us (-12%), with 20 headers 2.80us -> 2.08us
(-26%); about -425 bytes allocated per request in a keep-alive
hello-world load test. Behavior byte-identical; spec suite passes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011HBrMGfWRxTpYeEB4UT8Kf
warp: add end-to-end response-path benchmark
The existing bench suite (bench/Parser.hs) covers only request-line
parsing and header splitting; the entire response side (header
sanitization, indexing, Server/Date insertion, composition, chunking,
buffer management, timeout handling) was unmeasured.
Add warp:bench:response, a criterion benchmark that drives the real
sendResponse with a sink Connection (connSendAll discards), a real
time-manager Handle and a realistic indexed request header, so the
whole per-response path is measured end to end without socket IO.
Also benches composeHeader/indexRequestHeader/indexResponseHeader in
isolation. Compiled -threaded with the library's Strict/StrictData to
match production semantics.
Baseline on GHC 9.10.3 (this machine): builder response with 4 headers
1.58us, 20 headers 2.80us, 204-no-body 727ns.
Also sets benchmarks: True in cabal.project so the suite is built by
default.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011HBrMGfWRxTpYeEB4UT8Kf
Merge PR yesodweb#1087
upper bound for 'http-types'
Because I'm preparing for a major-major rework of headers, we should add upper bounds ASAP.
The packages that do not use headers from 'http-types' do not need an upper bound.
('wai' uses type synonyms only, which is forwards-compatible)
Merge pull request yesodweb#1088 from yesodweb/indexedheader-safety
Safer IndexedHeader
warp: bumped version and add to ChangeLog
warp: use a safer 'IndexedHeader' to avoid request/response mixups
warp: better API for 'IndexedHeader'
Using a phantom type parameter will give more guarantees that
the correct Enum is used. Otherwise, you could mix up
'RequestHeaderIndex' and 'ResponseHeaderIndex' without any
compile time errors.
This _DOES_ impact the API in 'Network.Wai.Handler.Warp.Internal'
but as it is prefaced to break easily, this shouldn't be a big problem.
bump version to 3.4.14 and add to ChangeLog
Merge pull request yesodweb#1084 from yesodweb/fix-waitForDecreased-at-zero-connections
Fix wait for decreased at zero connections
warp: fix 'waitForDecreased' locking on 0 connections
warp: give the result of checking the connections counter
ci: linux has 2 directories for GHCup for some reason?
ci: attempt to simplify 'Cache dependencies'
ci: use correct match for os and unique keys per GHC version
ci: trailing commas aren't allowed, I guess?
ci: no newlines in 'case'
ci: add ghcup binaries dir to cache, and 'APPDATA' dirs for Windows
ci: testing if we can maybe cache the executables too
ci: LTS-23 now also needs 'extra-deps'
wai-extra: comment/layout fix
ci: adjust 'stack.yaml/stack-nightly.yaml' to build with current stackage
Merge pull request yesodweb#1081 from alexfmpe/patch-1
Retroactively add note about threaded runtime to changelog
Retroactively add note about threaded runtime to changelog
Merge pull request yesodweb#1077 from yesodweb/fix-windows-not-working-with-waitReadSocketSTM
Fix windows not working with wait read socket stm
warp: update Changelog for 'network' adjustment
warp: add CPP to limit usage of 'waitReadSocketSTM'.
Merge pull request yesodweb#1078 from yesodweb/remove-Typeable-deriving
Remove typeable deriving
warp(-tls): same here
since 'wai(-extra)' actively supports only 'base >= 4.12', 'Typeable' is automatically derived
warp: forgot an extra 'pure'
warp: bumped patch version to be able to publish a new version that isn't actually different in code, except that it hopefully works on Windows again
warp: omit 'waitReadSocketSTM' when on WINDOWS
Merge pull request yesodweb#1072 from Bodigrim/patch-1
Add since annotation to connAppsInProgress
Add since annotation to connAppsInProgress
Merge pull request yesodweb#1071 from yesodweb/Vlix/graceful-shutdown
Vlix/graceful shutdown
Co-authored-by: Michał Kłeczek <michal@kleczek.org>
Co-authored-by: Kazu Yamamoto <kazu@iij.ad.jp>
warp: clarifying comment to 'Response.hs'
warp: move 'Counter' documentation below the new 'ServerState' and don't hard-set nightly builds to use GHC 9.12.2
warp: removing redundant exports from 'Internal'
* 'newServerState' is not used in 'warp-tls'
* other functions already get exported from regular 'Warp.hs'
warp: added disclaimer to 'Internal.hs' module to give potential users better expectations
warp: renamed to 'makeGracefulRecv' and added documentation
warp(-tls): changes according to code review
* Use 'Control.Concurrent.STM' instead of 'GHC.Conc' when possible
* Revert excessive diffs
* Add deprecation message to 'settingsConnectionCounter' documentation
* Better naming of boolean ('ok' -> 'isShuttingDown')
warp-tls: bump version and Changelog
warp-tls: adjust for 'warp-3.4.13'
warp: extra exports for 'warp-tls'
We don't want to expose 'newCounter' and 'ShuttingDown', so I've
changed the argument to 'makeRecv' to 'ServerState' and the type
of 'appsInProgress' to 'TVar Int'.
warp: revert 'socketConnection' to use 'ShuttingDown' from the 'Settings'
warp/test: added tests for 'ServerState'
warp: version bump and ChangeLog entry
warp: expose 'ServerState' selectively
The 'ServerState' should be READ-ONLY so that users can not break internal logic.
warp/test: add test for graceful shutdown
Co-authored-by: Michał Kłeczek <michal@kleczek.org>
relaxing boundary
Merge pull request yesodweb#1069 from edsko/edsko/stopAfterWithResult
Introduce `stopAfterWithResult`
warp: use 'ShuttingDown' from 'ServerState' and 'connAppsInProgress' to gracefully handle shutdown
warp: add 'ServerState' to 'Settings' as focus for queriable internals for users
! These should of course only ever be READ-ONLY !
warp: add 'ShuttingDown' module
warp: add 'getCountSTM'
Introduce `stopAfterWithResult`
workaround for cabal test
Merge pull request yesodweb#1063 from Vlix/fix/remove-crypton-cpp
Removed unreachable code from `wai-app-static`
wai-app-static: removed the unreachable code since the removal of 'crypton' as a dependency
relaxing boundaries
Merge branch 'remove-crypton'
wai-app-static: ver bumps up
removing crypton from wai-app-static
fix wai-app-static.cabal
Merge ram branch
wai-app-static: ver bumps up
using "ram" instead of "memory"
mime-types: also bumped version in cabal file -> 0.1.2.2
Merge pull request yesodweb#1059 from zoominsoftware/feat/mime-avif
mime-types: add image/avif
mime-types: add image/avif
Merge pull request yesodweb#1057 from yesodweb/time-manager-documentation-refactor
Time manager documentation refactor
time-manager: updated 'ChangeLog.md' with PR links
time-manager: bumping patch version, because the code does the same and just changed some documentation/comments
time-manager: lots of documentation additions/tweaks
time-manager: actually touching some code
time-manager: added constraints to 'base' dependency of test-suite
Merge pull request yesodweb#1056 from yesodweb/time-manager-tests
Time manager tests
hopefully fixes nightly
time-manager: added an extra test to mix up resume/pause
time-manager: finished tests for 'System.TimeManager'
minor adjustments
time-manager: moved 'getTimerManager' to 'Internal' module to also test "oldResume"
time-manager: first set of tests
time-manager: moved some definitions to 'Internal' module to add to test suite
Merge pull request yesodweb#1055 from yesodweb/time-manager-resume-fix
Time manager `resume` fix
bumped version and added to ChangeLog
added/adjusted comments and documentation
added a 'forkIO' to not block on throwing the exception
added state to the 'Handle' to adjust 'resume'
helper function to only act when handle is registered
s/return/pure/
allowing tls v2.2
Merge pull request yesodweb#1051 from ners/mime-javascript
mime-types: change type for JavaScript files to text/javascript
Merge pull request yesodweb#1052 from konsumlamm/master
Update source-repository sections & avoid `ghc-prim` dependency
Fix remaining source-repository sections
Merge pull request yesodweb#958 from wireapp/send-conn-close
warp: Send `Connection: close` when closing the connection
Update source-repository sections
Use https
Avoid ghc-prim dependency
Merge pull request yesodweb#1048 from kazu-yamamoto/system-timer-manager2
using GHC.Event.TimerManager
mime-types: change type for JavaScript files to text/javascript
warp: Add more HTTP/1.1 Connection: close tests
warp: Add test for Connection: close header behavior
warp: Update changelog
warp: Bump version to 3.4.12
warp: Send `Connection: close` when closing the conn
Merge pull request yesodweb#1050 from domenkozar/domenkozar/open-connection-count
warp: expose open connection count via getOpenConnectionCount
warp: expose open connection count via getOpenConnectionCount
Adds ability to monitor the number of currently open connections by
storing the connection counter in Settings and exposing it through
a new getOpenConnectionCount function.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
manual from @Vlix
typo (@Vlix)
changelog
feedback from @Vlix
updating doc
fixing broken functions
documentation
adding DEPRECATED
fix CI
warp: ver bumps up
warp: using newest dependencies
review from @Vlix
Manager 0 as defaultManager
allowing time-manager v0.3
time-manager: ver bumps up
changelog
time-manager: ver bumps up
exporting emptyHandle
used in http-semantics
noline
adding isAllGone
using http2 5.3.11
fourmolu
auto-update: add to changelog for documentation fix
auto-update: fixed some @SInCE notations
wai-extra: haven't published '3.1.18' yet, so combining changes and doing that now
seanparsons
marked this pull request as ready for review
July 20, 2026 13:21
Vlix
requested changes
Jul 24, 2026
Vlix
left a comment
Vlix
left a comment
Contributor
There was a problem hiding this comment.
The introduction of toBufIOWithOffset does introduce ways that future code changes could result in bad behaviour/errors/security issues. But since it's pretty local and not widely used, it should be fine.
Off-topic:
I also think we've overlooked the Chunk section, because the length of the bs there isn't counted towards the totalBytesSent... I think? I've dug through the ByteString.Builder source code to find how it works, and from what I can tell, the amount of bytes written would be 0, and the ByteString is just given... in which case the totalByteSent value would add len == 0, and then the Chunk is sent... without counting it???
A test would help us get this right.
I think I'll make a new issue for this.
- Rename toBufIOWithOffset to unsafeToBufIOWithOffset and turn its one-line offset note into a prominent warning spelling out the out-of-bounds consequences. - Document why the hdrLen < bufSize check in sendRsp is necessary for memory safety.
Contributor
|
Will continue with this PR and the others when I have time to also run the benchmarks. This PR and the others might have to be rebased since my PR might have changed how some things are handled. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of a series splitting #1090 into independently reviewable PRs; independent of the others.
For
RspBuilderresponses (responseBuilder/responseLBS, the most common type) the header block was copied twice: composed into a fresh pinned ByteString, then copied again into the connection write buffer (headers are nearly always < 4096 bytes, so bytestring's builder copies rather than inserts). Now the header is composed directly into the write buffer (composeHeaderPtr/composeHeaderLengthin ResponseHeader, sharing the existing copy loops) and the body builder continues after it viatoBufIOWithOffset(toBufIOWith = toBufIOWithOffset 0, no duplicated loop). Oversized headers fall back to the old path. The length reported to the logger is unchanged.Wire bytes verified byte-identical against master for content-length, chunked, and 20-header responses (only Date differing), including chunked framing. Measured:
sendResponse4 headers −6%, 20 headers −11%, ≈ −430 B/request. Spec suite passes.