Repository navigation
bench: measure both sides of every row on stdout+stderr - #3774
Conversation
01ae172 to
4cb5629
Compare
4cb5629 to
0950ceb
Compare
|
Hey @KuSh , thanks for this correctness fix in benchmark, LGTM Could you also install rg in the ci runner so benchmark pass ? all results look same length so I suspect a problem with rg execution in this environment. "tree" command has a guard for this, we could add an install step and the guard as well |
The raw side was measured with 2>&1 and the rtk side with 2>/dev/null, so the two columns were not the same measurement. rtk forwards the stderr it does not filter, and dropping that stream books it as if it had been removed. It also hid the failure mode this fixture is meant to catch. A filter that goes silent while the tool reports on stderr scored as a saving; the golangci-lint row read 157 -> - and failed the run for producing nothing, with no way to see that the diagnostic had gone to the other stream all along. For an identical measurement rtk is better or equal by construction: stderr is common to both sides, so never-worse reduces to `filtered <= stdout`. Measuring stderr also shows when the tool under test is missing. The CI runner has no rg, so the rg rows compared the shell's "command not found" with rtk's own error and went negative. The benchmark job now installs ripgrep, and the rg section is skipped when rg is absent, as the tree section already is. The run reports 0 negative and 0 fail both with rg installed (rg fn 125360 -> 4083) and without it (section skipped). The good/warn split moves by a row or two between machines and with what is installed, so it is not a property of this change. Verified in golang:1.27 that the go rows read 220 -> 35 GOOD on v2.13.2, and 157 -> 157 -- a tie, not a red run -- if the v2 install were ever reverted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0950ceb to
0f6b844
Compare
|
Thanks! Done in 0f6b844: the benchmark job now installs You were right about the cause. With stderr now measured on both sides, the rg rows were comparing the shell's I also rebased onto the current develop. |
The raw side of each benchmark row is measured with
2>&1and the rtk side with2>/dev/null, so the two columns are not the same measurement.rtk forwards the stderr it does not filter. Dropping that stream from the rtk column books a passed-through stream as if the filter had removed it, and it hides the failure mode the rows exist to catch: a filter that goes silent while the tool reports on stderr scores as a saving. The golangci-lint row reads
157 -> -and fails the run for producing nothing, with no way to see that the diagnostic went to the other stream all along.Why this cannot make rtk look worse
For an identical measurement rtk is better or equal by construction. stderr is common to both sides, so never-worse reduces to
filtered <= stdout.rg in CI
Measuring stderr also shows when the tool under test is missing. The CI runner has no
rg, so the rg rows compared the shell'srg: command not foundwith rtk's own error and went negative. The benchmark job now installsripgrepnext totree, and the rg section gets the samecommand -vguard as tree, so it is skipped when rg is absent.The run reports 0 negative and 0 fail both ways:
rg fn125431 → 4028,rg struct3434 → 2599,rg -l800 → 675,rg -c895 → 895The good/warn split moves by a row or two between machines and with what is installed locally, so it is not a property of this change and I have deliberately not quoted it as one.
Verified in
golang:1.27that the go rows read220 -> 35 GOODon golangci-lint v2.13.2, and157 -> 157— a tie, not a red run — if the v2 install were ever reverted.Independent of the other benchmark work
This does not depend on #3773 and does not change its numbers (
220 -> 35either way). It is a measurement-methodology change that touches every row, which is why it is separated out rather than folded in.It does read best after #3772: until stdout-only filters forward stderr at all, the golangci-lint row stays
FAILregardless of how it is measured.