perf(ab-testing): stream the fallback-proxy rewrite instead of buffering - #555
Open
JonasJesus42 wants to merge 1 commit into
Open
JonasJesus42 wants to merge 1 commit into
JonasJesus42 wants to merge 1 commit into
Conversation
`proxyToFallback` rewrites the fallback hostname out of the upstream body,
and did it with `await response.text()` — the entire body as a JS string
(two bytes per character) plus the replaced copy, resident at once. For a
multi-megabyte HTML page that is several MB of a 128MB isolate budget, to
substitute a hostname.
Now a TransformStream. Two hazards it has to handle, both covered:
- A match straddling a chunk boundary. Each pass holds back the longest
suffix of the buffer that is a proper prefix of the search string — and
nothing more. The obvious version of this (hold back a fixed
`search.length - 1`) is wrong in both directions: it can slice a
complete match in half, so neither side matches and the occurrence is
silently missed. That is not hypothetical; it was the first
implementation here and the tests caught it.
- A multi-byte character split across chunks — `TextDecoder` with
`{ stream: true }` carries the partial code point.
The held-back slice is taken from the raw input, never from replaced
output, so a replacement ending in a prefix of the search string cannot
join the next chunk and be replaced twice.
Two correctness fixes that came with it:
- A still-compressed body (`content-encoding` present) is now forwarded
untouched. A transform over gzip bytes produces garbage. Workerd strips
the header when it decompresses, so the streaming path still covers the
ordinary case.
- `content-length` is dropped when we rewrite. The substitution changes
the body length, so the upstream value was already a lie on the
buffered path — it just never had to be chunked.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Problema
proxyToFallbackreescreve o hostname de fallback no body do upstream, e fazia isso comawait response.text(): o body inteiro como string JS (2 bytes por caractere) mais a cópia trocada, residentes ao mesmo tempo. Para uma página HTML de alguns MB, são vários MB de um orçamento de 128 MB por isolate — para substituir um hostname. É o smell "buffered instead of streamed" que a skill de memória lista.Mudança
Vira
TransformStream. Dois perigos, os dois cobertos:Match na fronteira entre chunks. Cada passada segura só o maior sufixo do buffer que é prefixo próprio da string buscada — e nada além disso. A versão óbvia disso (segurar um
search.length - 1fixo) está errada nas duas direções: ela pode cortar um match completo ao meio, e aí nenhum dos lados casa e a ocorrência some em silêncio. Isso não é hipotético — foi a primeira implementação aqui e os testes pegaram.Caractere multi-byte partido entre chunks.
TextDecodercom{ stream: true }carrega o code point parcial.O pedaço segurado vem do input cru, nunca do output já substituído — senão uma substituição terminada em prefixo da busca poderia se juntar ao próximo chunk e ser trocada duas vezes.
Dois bugs de correção que vieram junto
content-encodingpresente) agora é encaminhado intacto. Transform sobre bytes gzip produz lixo. O workerd remove o header quando descomprime, então o caminho streamado cobre o caso normal; o que sobrar codificado passa direto em vez de ser decodificado só pra trocar um hostname.content-lengthé removido quando reescrevemos. A substituição muda o tamanho do body, então o valor do upstream já era mentira no caminho bufferizado — só que ali nunca precisou virar chunked.Testes
11 casos para o transform (fronteira, fronteira char-a-char, guard de dupla substituição, multi-byte partido, match no começo, no fim, body vazio, nada casando, corpo grande) + 3 no nível do proxy (streama e limpa o content-length, não encosta em body comprimido, não encosta em não-2xx).
🤖 Generated with Claude Code
Summary by cubic
Streams the fallback-proxy hostname rewrite through a
TransformStreaminstead of buffering the whole upstream body into a JS string, so memory use stays at one chunk plus a bounded tail. Compressed responses are now forwarded untouched, andcontent-lengthis removed on rewritten responses because the substitution changes the body length.Correctness notes
TextDecoderstreaming mode.Written for commit 6a5fa72. Summary will update on new commits.