benchmark: fix readable-hash-4mb pointing at a missing wrk script - #359
Merged
dimdenGD merged 1 commit intoJul 31, 2026
Merged
Conversation
The scenario asks for post-hash-body-4mb.lua but the file is named post-hash-body-25mb.lua, and its contents build a 4mb body, so it is the name that is wrong. With the script missing wrk falls back to its own defaults and requests GET / instead of posting anything, which is why that row reports 28.12k req/sec: at 4mb a request that would be 112 GB/sec of upload. It has been measuring a 404, not body streaming. Rename the file to what the scenario already refers to.
Contributor
Author
|
The benchmark job on this branch confirms it:
A drop from 28.12k to 209 req/sec is what starting to actually upload 4mb per request looks like. The suite had been reporting a 2.20x on a row that was requesting Worth knowing that the published number for that row has been wrong rather than merely optimistic. |
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.
streaming/readable-hash-4mbasks forpost-hash-body-4mb.lua, but the file inbenchmark/wrk-scripts/is calledpost-hash-body-25mb.lua. Its contents build a 4mb body, so the name is what is wrong, not the scenario.With the script missing, wrk falls back to its defaults and sends
GET /rather than posting a body, which means the row has never measured what it says it does. The published numbers give it away: 28.12k req/sec onmainwould be 112 GB/sec of upload at 4mb per request, and the 10.41 MB/sec transfer works out to 388 bytes per response, which is a 404 page rather than a sha256 digest. For comparison,streaming/writable-with-content-length, which really does move large payloads, sits at 562 req/sec.Renaming the file to the name the scenario already uses is enough. The row will get much slower after this, but it will finally be measuring the request-streaming path.
Found while looking at why body-related changes are invisible in this suite (see #358).