write() streams, appendBody() buffers, and five names that misreported (#180) - #182
Merged
Conversation
send() is removed rather than kept as an alias: it would have covered one call in the shipped laravel-spawn adapter while isClosed(), removed in the same release, breaks three others beside it. isClosed() becomes isEnded(); sendable() keeps its declaration and throws, naming isWritable() and tryWrite()/awaitWritable(), because those two are not guessable from the name; getBodyStream()/setBodyStream() are gone, neither having had an implementation. The stream perf profile called send() with no argument against an arginfo requiring one, so it answered 500 before measuring anything.
setBody()/appendBody()/json()/html() followed by a streaming call put only the streamed chunks on the wire: the streaming path commits its own headers and the buffered dispose path runs only while streaming is false. The reverse direction has always thrown, so the failure reported on one side only. The guard every streaming entry point shares now refuses a non-empty buffer and names both modes. An empty one does not count.
"waits for the write exactly as write() does" carried no fact after the rename. The sendable() tombstone says why the declaration outlives the method instead of restating what an undefined-method fatal looks like.
…#179) Release PHP built for the run, three wrk runs per cell, median. strace confirms three write(2) and three loop turns per chunk, flat in the chunk size; a coalesced frame halves the per-chunk cost. The win needs no queue, no second writer and no per-response structure, so it is not an argument for the design #179 proposes.
Contributor
CoverageTotal lines: 82.22% → 82.12% (-0.10 pp)
❌ Regression in touched files (> 1.0 pp drop)
Add |
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
1 participant
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.
Closes #180. Closes #181.
The renames (#180)
write()appended to a buffer and put nothing on the wire; the call thatstreamed was
send(). Node, Swoole and Go stream under the first name, and bothfield reports behind this contract work (YanGusik/laravel-spawn#50, #60) came
from the API rather than from the adapter's code.
send(string): staticwrite(string): staticwrite(string): staticappendBody(string): staticisClosed(): boolisEnded(): boolsendable(): boolisWritable()andtryWrite()/awaitWritable()getBodyStream(),setBodyStream()send()is removed rather than kept as an alias. It would have covered one callin the shipped laravel-spawn adapter (
src/Server/TrueAsyncServer.php:492) whileisClosed()breaks three others beside it at lines 106, 395 and 413 — theadapter needs a release either way, so a second spelling of
write()postponesthe same edit and carries a duplicate name through a minor.
sendable()keepsits declaration because its two replacements are not guessable from the name,
which is the one thing a tombstone is good for.
docs/USAGE.mdgains §3.5, the three body modes with the state each commits andhow each is framed — the section deferred from #174 so it could be written once
against the final names.
The buffered body was discarded without error (#181)
setBody()/appendBody()/json()/html()followed by a streaming call putonly the streamed chunks on the wire. The streaming path commits its own headers
and never reads
response->body, while the buffered dispose path runs onlywhile
streamingis false. The reverse direction has always thrown, so asymmetric-looking mistake reported on one side only. The guard every streaming
entry point shares now refuses a non-empty buffer and names both modes; an empty
one does not count, since
setBody('')commits the handler to nothing.Two things found in passing
tests/perf/servers/server_stream.phpcalled->send()with no argument againstan arginfo requiring one, so the
streamperf profile answered 500 withexpects exactly 1 argument, 0 givenbefore measuring anything, and its chunkloop buffered through the old
write(). Measured on the old file: 500 and 66bytes; on the new one: 200 and 65536.
examples/sse-server.phpbroke its loop on!sendable()— the same read asliveness that truncated the stream in #60, and which #174 removed from
README.md. It stops on!isWritable()now.Evidence
tests/phpt/server/core/062-body-api-names.phptreads the wire for each mode:/bufferedanswersContent-Length: 7,/two-chunksand/streamedanswerchunked,
/mixedanswersContent-Length: 8with the buffered body. Without the#181 guard
/mixedanswers chunked with bodystreamed— the failing run is inthe commit message trail.
h2/023-h2-sendable-tombstone.phptasserts the throwon a live HTTP/2 stream with a filling ring, which is where
sendable()used toreturn a meaningful answer; its former coverage of a refusal plus a byte-exact
hash is
h2/025-h2-try-write.phptin full.tests/phpt/server/+tests/phpt/websocket/: 346 passed, 0 failed, 24 skipped.