diff --git a/dev/BENCHMARKS.md b/dev/BENCHMARKS.md index cfd0e05..39efa90 100644 --- a/dev/BENCHMARKS.md +++ b/dev/BENCHMARKS.md @@ -48,6 +48,48 @@ three things both rejected designs foundered on. Whatever else #179 wants, a non-blocking `tryWrite()` on HTTP/1, has to be argued on its own; this measurement does not support it. +### Where the copy stops paying + +Coalescing costs one user-space copy of the chunk, so it pays only while that copy +is cheaper than the two syscalls it removes. Body held at 1 MiB, chunk size moved, +five runs per cell, median: + +| chunk | three writes | one write | gain | +|---|---|---|---| +| 1 KiB | 49 | 132 | +167% | +| 4 KiB | 226 | 374 | +65% | +| 16 KiB | 857 | 1319 | +54% | +| 32 KiB | 1486 | 1860 | +25% | +| 64 KiB | 2606 | 2197 | −16% | +| 128 KiB | 3616 | 3206 | −11% | +| 256 KiB | 4457 | 3546 | −20% | + +The crossing is between 32 and 64 KiB, so `H1_CHUNK_COALESCE_MAX` is 32 KiB and a +larger chunk keeps the three-write path. + +### The shipped change, verified against the noise + +Runs of the same build drift by up to 9% on this machine, which is wider than some +of the gains above, so the shipped change was re-measured by alternating the two +builds — start, three `wrk` runs, stop, swap — three rounds each. + +| chunk | three writes | shipped | | +|---|---|---|---| +| 64 KiB | 2362, 2409, 2511 | 2473, 2573, 2577 | same code path either side of the threshold; the spread is the noise floor | +| 4 KiB | 197, 243, 208 | 367, 376, 377 | +81%, and every run of one build is outside the other's range | + +### The header block in the same frame + +The first `write()` sent the status line and headers, then the frame. Carrying +the block inside the frame removes one write and one round trip from the byte a +client waits for. Alternating builds again, three rounds, median of three runs +each: + +| response | frame only | headers in the frame | | +|---|---|---|---| +| one 4 KiB chunk | 22981, 23868, 23387 | 30057, 31601, 29732 | +28.5%, the shape an SSE response has | +| one 64 KiB chunk | 16738, 16565, 16709 | 16376, 16657, 16477 | unchanged: the block is small against the frame | + ## 2026-08-19 — cost of the per-chunk flush on a streamed response (#170) Base commit 22a8d37 plus the #170 working tree. Machine: WSL2, Linux 6.6.114.1, diff --git a/dev/PLAN.md b/dev/PLAN.md index 5872a30..b131285 100644 --- a/dev/PLAN.md +++ b/dev/PLAN.md @@ -140,8 +140,16 @@ designs were worked out and both fail on something mechanical. `io_pipe_writev_cb` sends no NOTIFY and frees the request itself. What it decides: the win needs no queue, no second writer and no per-response - structure, so it is not an argument for #179. Next step is to land the coalesced - frame as its own change, with the copy or with a new ext/async op. + structure, so it is not an argument for #179. + +- [x] **Send a streamed chunk as one write below 32 KiB.** Done. The threshold is + where the copy stops paying, measured: +25% at a 32 KiB chunk, −16% at 64 KiB, so + a larger chunk keeps the copy-free three-write path. Verified against a 9% noise + floor by alternating the two builds — +81% at 4 KiB chunks, and no difference at + 64 KiB, where both take the same path. Test + `tests/phpt/server/h1/029-h1-chunk-coalesce.phpt` reads the raw response and + checks the chunk-size lines on both sides of the threshold. The copy stays until + ext/async gains an awaitable vectored write. - [ ] **Answer from the queues the connection already has.** Plaintext: `out_pending_buf` carries a byte count, a high-water predicate on the same knob, diff --git a/src/http1/http1_stream.c b/src/http1/http1_stream.c index 0ed09ae..84d78bb 100644 --- a/src/http1/http1_stream.c +++ b/src/http1/http1_stream.c @@ -41,12 +41,33 @@ /* Maximum hex chunk-size line (16 hex digits for 64-bit len) + CRLF. */ #define H1_CHUNK_HEADER_MAX 18 -static bool h1_emit_headers_once(http1_request_ctx_t *ctx) +/* Largest frame — size line, body and CRLF together — that goes out as one + * copied write instead of three separate ones. Coalescing trades two syscalls + * and two scheduler round-trips against one copy of the chunk, and the two are + * worth the same somewhere between 32 and 64 KiB: measured at a 1 MiB body, + * +25% at a 32 KiB chunk and -16% at 64 KiB (dev/BENCHMARKS.md, 2026-08-20, + * plaintext, wrk on loopback — the crossing moves with the machine). + * + * The bound is on the frame and not on the chunk because of TLS, where + * tls_push splits anything larger than the plaintext ring: a frame one byte + * over spends a second ring cycle on a TLS record carrying six bytes. The two + * numbers are independent — one is a measured crossing, the other a buffer + * size — so the assert below catches them drifting apart rather than tying + * the plaintext decision to a TLS constant. */ +#define H1_CHUNK_COALESCE_MAX (32 * 1024) + +ZEND_STATIC_ASSERT(H1_CHUNK_COALESCE_MAX <= HTTP_TLS_PLAINTEXT_RING_BYTES, + "a coalesced frame must fit one TLS plaintext ring cycle"); + +/* The status line and headers of a streaming response, as bytes. Returns NULL + * when the response is gone or formats to nothing; the caller owns the string. + * Separate from the send so the first frame can carry the block with it. */ +static zend_string *h1_streaming_headers_build(http1_request_ctx_t *ctx) { http_connection_t *conn = ctx->conn; if (Z_ISUNDEF(ctx->response_zv)) { - return false; + return NULL; } zend_object *response_obj = Z_OBJ(ctx->response_zv); @@ -66,15 +87,33 @@ static bool h1_emit_headers_once(http1_request_ctx_t *ctx) zend_string *headers = http_response_format_streaming_headers(response_obj); - if (headers == NULL || ZSTR_LEN(headers) == 0) { - if (headers != NULL) { - zend_string_release(headers); - } + if (headers != NULL && ZSTR_LEN(headers) == 0) { + zend_string_release(headers); + return NULL; + } + + return headers; +} + +/* Headers reached the wire: the response is a streaming one from here. + * H2 and H3 count it on their first chunk too; without the counter an H1 + * stream showed up in the send/byte totals but never in + * streaming_responses_total. */ +static void h1_stream_headers_committed(http1_request_ctx_t *ctx) +{ + ctx->h1_stream_headers_sent = true; + http_server_on_streaming_response_started(ctx->conn->counters); +} + +static bool h1_emit_headers_once(http1_request_ctx_t *ctx) +{ + zend_string *headers = h1_streaming_headers_build(ctx); + if (headers == NULL) { return false; } - const bool ok = http_connection_send(conn, ZSTR_VAL(headers), + const bool ok = http_connection_send(ctx->conn, ZSTR_VAL(headers), ZSTR_LEN(headers)); zend_string_release(headers); return ok; @@ -109,33 +148,49 @@ static int h1_stream_append_chunk(void *opaque, zend_string *chunk, return HTTP_STREAM_APPEND_STREAM_DEAD; } - /* First write() — commit status + headers with chunked framing. + /* First write() — commit status + headers with chunked framing. The block + * is built here and sent with the frame below, so time-to-first-byte costs + * one write rather than two; that matters most for SSE, where the first + * event is the whole point and is a few dozen bytes. + * * We track wire-commit on ctx->h1_stream_headers_sent rather than * response->committed because write() sets committed=true before * calling us (committed means "no more setHeader / setStatusCode * allowed", which happens at the PHP boundary, not on the wire). */ + zend_string *headers = NULL; + if (!ctx->h1_stream_headers_sent) { - if (!h1_emit_headers_once(ctx)) { + headers = h1_streaming_headers_build(ctx); + + if (headers == NULL) { ctx->stream_dead = true; zend_string_release(chunk); return HTTP_STREAM_APPEND_STREAM_DEAD; } - - ctx->h1_stream_headers_sent = true; - - /* Headers on the wire = this response is now a streaming one. H2/H3 - * count it on their first chunk too; without this an H1 stream showed - * up in the send/byte counters but never in streaming_responses_total. */ - http_server_on_streaming_response_started(conn->counters); } /* Empty chunk is legal on the wire but would be indistinguishable * from the zero-chunk EOF marker — drop it silently. mark_ended() - * is the only place that emits the zero-chunk. */ + * is the only place that emits the zero-chunk. It still commits the + * headers, which is what a handler opening a stream with one expects. */ const size_t chunk_len = ZSTR_LEN(chunk); if (chunk_len == 0) { zend_string_release(chunk); + + if (headers != NULL) { + const bool sent = http_connection_send(conn, ZSTR_VAL(headers), + ZSTR_LEN(headers)); + zend_string_release(headers); + + if (!sent) { + ctx->stream_dead = true; + return HTTP_STREAM_APPEND_STREAM_DEAD; + } + + h1_stream_headers_committed(ctx); + } + http_server_on_stream_send(conn->counters, 0); return HTTP_STREAM_APPEND_OK; } @@ -146,12 +201,73 @@ static int h1_stream_append_chunk(void *opaque, zend_string *chunk, if (header_len < 0 || (size_t)header_len >= sizeof(header)) { zend_string_release(chunk); + + if (headers != NULL) { + zend_string_release(headers); + } + return HTTP_STREAM_APPEND_STREAM_DEAD; } - if (!http_connection_send(conn, header, (size_t)header_len) || - !http_connection_send(conn, ZSTR_VAL(chunk), chunk_len) || - !http_connection_send(conn, "\r\n", 2)) { + /* One write per frame while the copy is cheaper than the two syscalls it + * removes; a large chunk keeps the three-write path and stays copy-free. + * Each http_connection_send suspends the handler until its write + * completes, so the count of them is the count of scheduler round-trips. + * + * Both branches hand a buffer the caller owns to a write that outlives the + * call when a cancellation lands mid-flight: libuv keeps the pointer until + * its completion callback, while dispose only marks the request pending. + * Closing that needs a write which reports its status AND takes the buffer + * over — today's ABI offers one or the other, never both. */ + const size_t head_len = headers != NULL ? ZSTR_LEN(headers) : 0; + const size_t frame_len = head_len + (size_t)header_len + chunk_len + 2; + const bool coalesce = frame_len <= H1_CHUNK_COALESCE_MAX; + bool frame_ok = true; + + /* Too large to carry the block along: the headers go out on their own, and + * the commit is recorded the moment they land rather than after the frame. + * Anything else lets a failure in between look like headers that were + * never sent, and mark_ended would send them a second time. */ + if (headers != NULL && !coalesce) { + frame_ok = http_connection_send(conn, ZSTR_VAL(headers), head_len); + + if (frame_ok) { + h1_stream_headers_committed(ctx); + } + } + + if (frame_ok && coalesce) { + char *const frame = emalloc(frame_len); + char *at = frame; + + if (headers != NULL) { + memcpy(at, ZSTR_VAL(headers), head_len); + at += head_len; + } + + memcpy(at, header, (size_t)header_len); + at += header_len; + memcpy(at, ZSTR_VAL(chunk), chunk_len); + at += chunk_len; + memcpy(at, "\r\n", 2); + + frame_ok = http_connection_send(conn, frame, frame_len); + efree(frame); + + if (frame_ok && headers != NULL) { + h1_stream_headers_committed(ctx); + } + } else if (frame_ok) { + frame_ok = http_connection_send(conn, header, (size_t)header_len) + && http_connection_send(conn, ZSTR_VAL(chunk), chunk_len) + && http_connection_send(conn, "\r\n", 2); + } + + if (headers != NULL) { + zend_string_release(headers); + } + + if (!frame_ok) { /* The write is how the peer's departure becomes visible on H1 — record * it so isWritable() can answer without a second doomed write. */ ctx->stream_dead = true; @@ -159,6 +275,7 @@ static int h1_stream_append_chunk(void *opaque, zend_string *chunk, return HTTP_STREAM_APPEND_STREAM_DEAD; } + /* Each of those writes suspends. A cancellation that lands between them * returns success for the writes already done, leaving the frame partly * written — the same state a failure leaves, and it is recorded the same @@ -187,6 +304,19 @@ static void h1_stream_mark_ended(void *opaque) http_connection_t *conn = ctx->conn; + /* A frame can be left half on the wire: above the coalescing threshold it + * is three writes with a suspension between them, and a cancellation lands + * in one of those gaps. Sealing that with a terminal chunk would tell the + * peer the body ended cleanly and hand the connection on for reuse, and it + * would read the terminator as the first bytes of the chunk the size line + * promised. Refuse everything from here: no headers, no terminator, no + * keep-alive. Checked before the header commit below, so a stream that + * died after its headers landed does not send them twice. */ + if (UNEXPECTED(ctx->stream_dead)) { + conn->keep_alive = false; + return; + } + /* If write() was never called but mark_ended fires anyway (rare: * handler flipped streaming mode then immediately closed), we * still need to commit the headers so the peer isn't left @@ -196,18 +326,7 @@ static void h1_stream_mark_ended(void *opaque) return; } - ctx->h1_stream_headers_sent = true; - } - - /* A chunk is three writes — size line, body, CRLF — with a suspension - * between them, so a cancellation can leave a frame half on the wire. - * Sealing that with a terminal chunk would tell the peer the body ended - * cleanly and hand the connection on for reuse, and it would read the - * terminator as the first bytes of the chunk the size line promised. - * Refuse both: no terminator, no keep-alive. */ - if (UNEXPECTED(ctx->stream_dead)) { - conn->keep_alive = false; - return; + h1_stream_headers_committed(ctx); } /* Terminal zero-chunk. Trailers not emitted — RFC requires the diff --git a/tests/phpt/server/compression/052-h1-streaming-backpressure-api.phpt b/tests/phpt/server/compression/052-h1-streaming-backpressure-api.phpt new file mode 100644 index 0000000..506076a --- /dev/null +++ b/tests/phpt/server/compression/052-h1-streaming-backpressure-api.phpt @@ -0,0 +1,112 @@ +--TEST-- +Compression H1 streaming: isWritable/tryWrite/awaitWritable answer from the transport, not the wrapper +--EXTENSIONS-- +true_async_server +true_async +--SKIPIF-- + +--FILE-- +addListener('127.0.0.1', $port) + ->setReadTimeout(5)->setWriteTimeout(5)); + +$payload = str_repeat("compressible line for the backpressure probe\n", 200); +$probe = []; + +$server->addHttpHandler(function ($req, $res) use ($payload, &$probe, $server) { + $res->setHeader('Content-Type', 'text/plain'); + + $q = (int)(strlen($payload) / 4); + + $res->write(substr($payload, 0, $q)); + $probe['writable_after_first'] = $res->isWritable(); + + $probe['try_accepted'] = $res->tryWrite(substr($payload, $q, $q)); + $probe['await_ready'] = $res->awaitWritable(1000); + + $res->write(substr($payload, 2 * $q, $q)); + $res->end(substr($payload, 3 * $q)); + + $probe['ended'] = $res->isEnded(); + $probe['writable_end'] = $res->isWritable(); + + $server->stop(); +}); + +$cli = spawn(function () use ($port, $payload) { + usleep(30000); + $fp = stream_socket_client("tcp://127.0.0.1:$port", $errno, $errstr, 5); + stream_set_timeout($fp, 5); + fwrite($fp, "GET / HTTP/1.1\r\nHost: x\r\nAccept-Encoding: gzip\r\n" + . "Connection: close\r\n\r\n"); + + $wire = ''; + while (!feof($fp)) { + $c = fread($fp, 65536); + if ($c === '' || $c === false) break; + $wire .= $c; + } + fclose($fp); + + [$head, $rest] = explode("\r\n\r\n", $wire, 2); + echo "gzip=", (int)(bool)preg_match('/^content-encoding:\s*gzip/mi', $head), "\n"; + echo "chunked=", (int)(bool)preg_match('/^transfer-encoding:\s*chunked/mi', $head), "\n"; + + /* De-chunk, then inflate. */ + $body = ''; + $off = 0; + while (true) { + $eol = strpos($rest, "\r\n", $off); + if ($eol === false) break; + $len = hexdec(substr($rest, $off, $eol - $off)); + $off = $eol + 2; + if ($len === 0) break; + $body .= substr($rest, $off, $len); + $off += $len + 2; + } + + $plain = @gzdecode($body); + echo "decoded=", (int)($plain === $payload), "\n"; + echo "smaller=", (int)(strlen($body) < strlen($payload)), "\n"; +}); + +$server->start(); +await($cli); + +foreach ($probe as $k => $v) echo "$k = ", var_export($v, true), "\n"; +echo "done\n"; +?> +--EXPECT-- +gzip=1 +chunked=1 +decoded=1 +smaller=1 +writable_after_first = true +try_accepted = true +await_ready = true +ended = true +writable_end = false +done diff --git a/tests/phpt/server/h1/029-h1-chunk-coalesce.phpt b/tests/phpt/server/h1/029-h1-chunk-coalesce.phpt new file mode 100644 index 0000000..1738ce7 --- /dev/null +++ b/tests/phpt/server/h1/029-h1-chunk-coalesce.phpt @@ -0,0 +1,137 @@ +--TEST-- +HttpResponse::write() — chunk framing holds either side of the coalescing threshold +--EXTENSIONS-- +true_async_server +true_async +--FILE-- + [1024, 32 * 1024, 32 * 1024 + 1, 64 * 1024], + '/large' => [64 * 1024, 512], + '/empty' => [0, 64], +]; + +$body = function (array $sizes): string { + $out = ''; + foreach ($sizes as $i => $n) { + $out .= str_repeat(chr(65 + $i), $n); + } + return $out; +}; + +$server = new HttpServer((new HttpServerConfig()) + ->addListener('127.0.0.1', $port) + ->setReadTimeout(10)->setWriteTimeout(10)); + +$server->addHttpHandler(function ($req, $res) use ($plan, $server) { + $sizes = $plan[$req->getPath()] ?? []; + + $res->setStatusCode(200)->setHeader('Content-Type', 'application/octet-stream'); + + foreach ($sizes as $i => $n) { + $res->write($n === 0 ? '' : str_repeat(chr(65 + $i), $n)); + } + + $res->end(); + + if ($req->getPath() === '/empty') { + $server->stop(); + } +}); + +$fetch = function (int $port, string $path): array { + $fp = stream_socket_client("tcp://127.0.0.1:$port", $errno, $errstr, 5); + stream_set_timeout($fp, 5); + fwrite($fp, "GET $path HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + + $wire = ''; + while (!feof($fp)) { + $c = fread($fp, 65536); + if ($c === '' || $c === false) break; + $wire .= $c; + } + fclose($fp); + + return explode("\r\n\r\n", $wire, 2); +}; + +$cli = spawn(function () use ($port, $plan, $body, $fetch) { + usleep(30000); + foreach ($plan as $path => $sizes) { + [$head, $rest] = $fetch($port, $path); + $expected = $body($sizes); + $sizes = array_values(array_filter($sizes)); + echo "== $path\n"; + echo "chunked=", (int)(bool)preg_match('/^transfer-encoding:\s*chunked/mi', $head), "\n"; + + /* Walk the chunked body by hand: every size line must match what the + * handler wrote, in order, and the terminator must be the last thing. */ + $read = ''; + $seen = []; + $off = 0; + while (true) { + $eol = strpos($rest, "\r\n", $off); + if ($eol === false) { echo "TRUNCATED\n"; break; } + $len = hexdec(substr($rest, $off, $eol - $off)); + $off = $eol + 2; + if ($len === 0) break; + $seen[] = $len; + $read .= substr($rest, $off, $len); + $off += $len + 2; + } + + echo "sizes_match=", (int)($seen === $sizes), "\n"; + echo "sizes=", implode(',', $seen), "\n"; + echo "body_len=", strlen($read), "\n"; + echo "body_match=", (int)($read === $expected), "\n"; + } +}); + +$server->start(); +await($cli); +echo "done\n"; +?> +--EXPECT-- +== /small +chunked=1 +sizes_match=1 +sizes=1024,32768,32769,65536 +body_len=132097 +body_match=1 +== /large +chunked=1 +sizes_match=1 +sizes=65536,512 +body_len=66048 +body_match=1 +== /empty +chunked=1 +sizes_match=1 +sizes=64 +body_len=64 +body_match=1 +done diff --git a/tests/phpt/server/h3/047-h3-reactor-pool-streaming.phpt b/tests/phpt/server/h3/047-h3-reactor-pool-streaming.phpt index 1c2e9b7..f015d4e 100644 --- a/tests/phpt/server/h3/047-h3-reactor-pool-streaming.phpt +++ b/tests/phpt/server/h3/047-h3-reactor-pool-streaming.phpt @@ -18,7 +18,12 @@ PHP_HTTP3_DISABLE_RETRY=1 * worker posts STREAM_HEADERS on first call (streaming submit on the * reactor), each chunk as STREAM_CHUNK (reactor chunk ring + resume), and * end() as STREAM_END (EOF). Before this, write() under the pool threw - * "streaming not available". Chunk ORDER and CONTENT prove the FIFO wire. */ + * "streaming not available". Chunk ORDER and CONTENT prove the FIFO wire. + * + * The backpressure trio rides along on the same response, because the pool is + * the only path where their answers come from a credit the reactor holds + * rather than from a queue the worker can see: isWritable() reads the credit, + * tryWrite() posts without waiting on it, and awaitWritable() waits for it. */ use TrueAsync\HttpServer; use TrueAsync\HttpServerConfig; @@ -52,6 +57,11 @@ $server->addHttpHandler(function ($req, $res) { $res->write("chunk{$i};"); } + $alive = $res->isWritable(); + $try = $res->tryWrite('chunk6;'); + $room = $res->awaitWritable(1000); + $res->write('probe:' . (int)$alive . (int)$try . (int)$room . ';'); + $res->end(); }); @@ -79,5 +89,5 @@ $server->start(); ?> --EXPECTF-- %Astatus=200 -body=chunk1;chunk2;chunk3;chunk4;chunk5; +body=chunk1;chunk2;chunk3;chunk4;chunk5;chunk6;probe:111; %A