Repository navigation
Benchmark cookie cost and refresh the per-request numbers - #6
Merged
Merged
Conversation
The cost benchmark measured cookie-less requests only, so it never exercised
the path that dominated real traffic: CRS resolves REQUEST_COOKIES on 162 rules
with a `!REQUEST_COOKIES:/__utm/` exclusion, and before selectors were compiled
once that recompiled a regex per cookie per rule per request. A browser request
with nine cookies cost 1.6 ms; twenty-four cost 3.7 ms. The published table,
all cookie-free, showed none of it.
Adds two cookie cases (a typical browser jar, and a heavy one) and wires
bench_inspect as a binary, which its own header already documented but the
manifest never declared.
Refreshes the README table with current numbers on the same machine. Selectors
are now compiled once and inspected values are tested in place rather than
copied, so cookie-free requests are several times cheaper and cookie-bearing
requests no longer scale with a per-cookie recompile:
GET, no query 313 -> 52 us
GET, browser cookies 1598 -> 277 us
GET, 24 cookies 3681 -> 604 us
Drops the stale in-gateway figure and the "500 req/s" and "7% copy attempt"
lines: the in-gateway number was measured against the old engine and a
deployment should measure its own.
A second [[bin]] made cargo run -p parapet-conformance ambiguous, and the conformance CI jobs invoke it without --bin, so they failed to run (the test job only builds, so it stayed green). An [[example]] keeps the package's single default binary and leaves those jobs untouched.
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.
Why
Answering "do we need to redo the benchmark and update the docs?" after the selector-precompile (#3) and copy-avoidance (#4) changes. Yes to both, and the benchmark had a blind spot worth fixing.
The blind spot
bench_inspectmeasured cookie-less requests only. But CRS resolvesREQUEST_COOKIESon 162 rules, each with a!REQUEST_COOKIES:/__utm/exclusion, and before selectors were compiled once that recompiled a regex per cookie, per rule, per request. So the one path that dominated real (browser) traffic was the one the benchmark never touched. The README's "1.9 ms in a real gateway vs ~0.3 ms isolated" gap was precisely this: live requests carry cookies, the benchmark didn't.Same-machine A/B (added cookie cases, run against the pre-fix and current engine)
The pre-fix "before" GET-no-query (313 µs) matches the README's old 311 µs, confirming the same machine. Two distinct wins: cookie-free requests got 1.6–6× from testing values in place instead of copying every one (#4, pure allocation churn on requests where regexes mostly reject); cookie requests got 5.8–6.1× from that plus compiling selectors once (#3). The 3.7 ms pre-fix 24-cookie case confirms the microbenchmark that motivated #3.
Changes
__utm*/_pk_ref, and a 24-cookie jar).bench_inspectas a[[bin]], which its own header documented (cargo run --bin bench_inspect) but the manifest never declared.Not touched: barbacane's docs
barbacane pins an old parapet rev, so its shipped gateway still has the slow path and its
docs/guide/waf.md"~2 ms per request" is still accurate for what it ships today. Updating it before bumping the parapet rev would claim a speed the binary doesn't have. The correct sequence there is: bump the rev, re-measure in-gateway, then update the barbacane doc, as a separate change.