Route brotli through the compression-rs autogated router - #7158
Open
guybedford wants to merge 2 commits into
Open
Route brotli through the compression-rs autogated router#7158guybedford wants to merge 2 commits into
guybedford wants to merge 2 commits into
Conversation
The unprefixed brotli symbol names (BrotliEncoderCompressStream,
BrotliDecoderDecompressStream, ...) are now owned by a compiled routing
layer, src/workerd/util/brotli-router.c++, which forwards each call to
one of the two implementations linked into the binary: the C brotli
library (rebuilt with the routed entry points renamed to brotli_c_* by
build/BUILD.brotli) or rust-brotli (exported under brotli_rs_* prefixes
by src/rust/brotli-rs, with the crate's unprefixed ffi-api exports left
off). The branch is set process-globally from the compression-rs
autogate alongside the existing zlib router whenever the gate state
changes.
Since every consumer resolves the declarations in brotli/encode.h and
brotli/decode.h to the routing layer, the gate now covers node:zlib's
brotli modes, the shared Brotli{Encoder,Decoder}Context machinery behind
web streams, and the kj-brotli streams used for fetch content-encoding
and response-stream observation. @Brotli moves from the BCR module to
the upstream release archive with our own build file so the C
implementation can be compiled under prefixed names, mirroring
build/BUILD.zlib.
src/rust/brotli-rs implements the routed C API subset over the crates'
Rust APIs: allocation uses the Rust global allocator (the C allocator
callbacks are accepted and ignored) and decoder error strings match the
C library's. A new kj_test verifies the router selects the matching
implementation on both sides of the gate, and a new bench-brotli
benchmark measures the two implementations on the kj-http streaming
workload (rust-brotli currently ~25% slower on compress at quality 5 and
~35% slower on decompress).
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
…typedef The brotli C API permits a null next_out with zero available_out (and null next_in with zero available_in; BrotliContext::clearBuffers produces exactly that), so only advance the cursors when bytes were actually consumed or written.
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.
This extends the compression-rs autogate from zlib to brotli, using the same routing module technique in #7132.
This results in both the
Content-Encoding: brconfiguration, and Node.js usage delegating to the Rust version when the autogate is enabled.Headline numbers, measuring the fetch
Content-Encoding: brconfiguration (quality 5, lgwin 19, 1MB compressible data in 16KB chunks, opt build):With the latest Rust nightly and SIMD features of the Rust brotli implementation (not yet included in this PR) we get:
The main question here is thus whether we are okay with a 25-30% slowdown, with the SIMD and newer nightly Rust / LLVM closer to 20%. Hopefully that improves further over time as well.
For node:zlib the gap depends on quality: at node's default q11 the one-shot compress measures ~10-18% slower on 1MB inputs, with the q4/q5 range matching the streaming numbers above. Compressed output sizes match the C library within noise at all measured qualities (worst case +2.7% at q5 on one input; q11 outputs effectively identical). The NPOSTFIX/NDIRECT encoder params are not supported by rust-brotli's encoder and report failure under the gate; decoder params are accepted and ignored.
The unprefixed
BrotliEncoder*/BrotliDecoder*symbol names are now owned by a compiled routing layer,src/workerd/util/brotli-router.c++, which forwards each call to one of the two implementations linked into the binary: the C brotli library (rebuilt with the routed entry points renamed tobrotli_c_*bybuild/BUILD.brotli) or rust-brotli (exported underbrotli_rs_*prefixes bysrc/rust/brotli-rs). The branch is set process-globally from the compression-rs autogate alongside the existing zlib router.Every consumer resolves the
brotli/encode.h/brotli/decode.hdeclarations to the routing layer, so the gate covers fetchContent-Encoding: brencoding/decoding and response-stream observation (kj-brotli), and node:zlib's brotli modes (the sharedBrotli{Encoder,Decoder}Contextmachinery).@brotlimoves from the BCR module to the upstream release archive with our own build file so the C implementation can be compiled under prefixed names, mirroringbuild/BUILD.zlibsrc/rust/brotli-rsimplements the routed C API subset over thebrotli/brotli-decompressorcrates: allocation uses the Rust global allocator (the C allocator callbacks are accepted and ignored), decoder error strings are byte-identical to the C library's, and the crates' ffi-api feature stays off since it would export the unprefixed namesThe router test, node:zlib brotli tests, compression stream tests, and the full test suite pass in both gate states.