bryancall commented on PR #13376:
URL: https://github.com/apache/trafficserver/pull/13376#issuecomment-5770522820
## Benchmark: base vs head, measured
Separate from my review, here are A/B numbers for this PR. Posting them
because the change is 70 days old and a measured result is more useful than
another opinion.
### How the arms were built
The benchmark source only exists on this branch, so comparing head's
benchmark against anything else risks measuring the harness rather than the
parser. Both arms therefore carry **byte-identical benchmark source** and
differ only in `src/proxy/hdrs`:
- **BASE** = merge-base `9c69fe1bcc` plus one commit that is `git checkout
pr13376 -- tools/benchmark/benchmark_HdrParse.cc
tools/benchmark/CMakeLists.txt`. The **head** version of those files was taken
deliberately, not the version from `b00bdec54`, because `ccc437f95` later
rewrote the `--iters` argv parsing.
- **HEAD** = `7e9255ff4` as-is.
```
$ git diff --stat 9c69fe1bcc..base-with-bench
tools/benchmark/CMakeLists.txt | 6 +
tools/benchmark/benchmark_HdrParse.cc | 813 +++++++++++++++++++++
2 files changed, 819 insertions(+)
$ git diff --stat 9c69fe1bcc..base-with-bench -- src/proxy/hdrs/
include/proxy/hdrs/
(empty)
$ git diff --stat base-with-bench pr13376 -- tools/benchmark/
(empty)
```
Build and invocation, so this is repeatable:
```
cmake --preset release -DENABLE_BENCHMARKS=ON
-DBUILD_EXPERIMENTAL_PLUGINS=OFF
cmake --build build-release --target benchmark_HdrParse -j 28
taskset -c 4 ./build-release/tools/benchmark/benchmark_HdrParse "[bench]" \
--benchmark-samples 200 --order decl --rng-seed 1 --reporter xml
```
GCC 16.2.1, `CMAKE_CXX_FLAGS_RELEASE=-O3 -DNDEBUG`, no sanitizers, both arms
clean with zero warnings. Pinned to one core, fixed order and RNG seed, **7
rounds strictly interleaved base/head/base/head**. Load average during the runs
was 1.0 to 1.1, which is essentially just the pinned process.
### Results, ns/op, median of 7 interleaved runs
| case | base | head | delta | worst-arm spread |
|---|---:|---:|---:|---:|
| url: realistic | 217.38 | 51.66 | **-76.2%** | 13.6% |
| response: 50 dup fields | 2546.03 | 1558.11 | **-38.8%** | 3.0% |
| request: realistic (zero-copy) | 367.62 | 322.17 | **-12.4%** | 3.1% |
| request: realistic (copy) | 449.94 | 403.36 | **-10.3%** | 2.9% |
| request: modern browser (zero-copy) | 643.86 | 593.93 | **-7.8%** | 5.9% |
| mime: 50 wks-miss fields | 4142.33 | 3907.70 | -5.7% | 15.7% |
| request: 100 fields | 12344.90 | 12071.50 | -2.2% | 9.0% |
| response: realistic | 320.85 | 316.54 | -1.3% | 5.5% |
| mime: realistic fields | 295.04 | 291.39 | -1.2% | 3.9% |
| wks: tokenize all field names | 2124.73 | 2146.28 | **+1.0%** | 0.7% |
| wks: tokenize, lowercased (H2) | 2124.30 | 2147.43 | **+1.1%** | 0.8% |
Per-run figures for the cases that carry the result, so you can judge the
spread yourself:
- **url: realistic** base `191.2 192.9 194.6 217.4 218.9 220.5 220.7`, head
`50.6 51.2 51.5 51.7 52.0 53.0 53.1`. Base is visibly bimodal, two clusters
about 14% apart, which I did not chase down. Even against base's *fastest*
cluster the head is about 73% faster.
- **response: 50 dup fields** base `2509 2519 2530 2546 2549 2549 2585`,
head `1551 1554 1556 1558 1561 1561 1578`. No overlap at all.
- **request: realistic (zero-copy)** base `362.3 to 373.8`, head `317.4 to
325.7`. No overlap.
- **request: modern browser** base min 625.2 is above head max 603.6, so
despite base's 5.9% spread the distributions do not overlap.
- **mime: 50 wks-miss fields** base had one outlier run at 4654.9 inflating
the spread. Excluding it: base 4006 to 4252, head 3848 to 4028. The win is
likely real but I would not quote a precise figure.
The three cases the PR does not touch move 1 to 2%, which is inside noise.
That is the control you want to see.
### One measured regression
The standalone WKS tokenize entry point is consistently **slower**. Since 1%
is close to noise, those two cases were re-run alone, 10 interleaved rounds
each, 300 samples, on a different core:
```
taskset -c 6 ./benchmark_HdrParse "[wks]" --benchmark-samples 300 --order
decl --rng-seed 1 --reporter xml
```
- `wks: tokenize all field names` base median 2129.24 (2127.1 to 2137.8),
head median 2153.07 (2143.1 to 2157.6), **+1.12%, zero distribution overlap**
- `wks: tokenize, lowercased (H2 form)` base median 2129.38 (2125.2 to
2137.0), head median 2153.78 (2152.1 to 2179.7), **+1.15%, zero distribution
overlap**
So: a reproducible ~1.1% slowdown on standalone `hdrtoken_tokenize`, across
20 interleaved rounds on two different cores with no overlap between arms.
**Inferred rather than measured:** this is most likely the fused scan-and-hash
trading standalone tokenize cost for the fused parse path, which would be
consistent with the large wins on exactly the cases that go through that path.
I did not profile to confirm the attribution, so treat the cause as a
hypothesis and the effect as measured.
It is dwarfed by the wins and I am not asking for anything about it. It is
worth knowing it exists.
### Unit tests, Release build, head arm
`test_proxy_hdrs` passes: **457,223 assertions in 62 test cases**. Base for
reference: 456,999 assertions in 57 cases, so this PR adds 5 cases.
### What these numbers do not support
- **Real-world impact.** This is a microbenchmark of `src/proxy/hdrs` in
isolation. It says nothing about what fraction of a real transaction is header
parsing. No h2load or end-to-end proxy run was done.
- **Per-commit attribution.** Base against head only, not bisected. The URL
and dup-field attributions above are inferred from reading the diffs.
- **Other compilers.** GCC 16.2.1 only. The URL win depends on the compiler
auto-vectorizing the new branchless loop, so clang or an older GCC could differ
in magnitude. Your comment claims both clang and GCC vectorize at `-O3`; I
verified the GCC half by measurement rather than by reading the generated
assembly, and separately confirmed the clang half by compiling the loop
standalone at `-O3` for arm64 and x86-64, where `-Rpass=loop-vectorize` reports
vectorization width 16 in both.
- **Loaded behavior.** Single pinned core, single threaded, no cache or
memory-bandwidth pressure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
