-
Notifications
You must be signed in to change notification settings - Fork 17.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
x/perf/cmd/benchstat: units inconsistent with column header when time per op exceeds one second #61262
Comments
Can you show the input text files? As a casual (not regular) user of benchstat this confuses me each time I see it too. This doesn't happen with bytes/op because 50k seems about as clear as 50kB. |
(In my typical benchmarks I am by now used to seeing "5.2n" which I inferred meant nanoseconds the first time I saw it. But "n" does not have the ambiguity that "m" does.) |
Input files attached: |
It is indeed milliseconds rather than minutes. So that's a bit less surprising, at least. 😅 |
That's an interesting ambiguity with "minutes". Benchstat would never show something in "minutes", but I can see how it would be confusing.
I'm not sure I understand this. The "sec/op" column will always say "sec/op", regardless of the scale of the values. The cells do use a common scale across a row, but not within a column, so there's not a clear way to move the scale suffix into the column header. (Usually the rows are different benchmarks, which might have very different scales.) |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
What did you expect to see?
Column headers consistent with the data within each column.
What did you see instead?
Note that the column headers report
sec/op
, but then the column data includes anm
suffix, suggesting either minutes or milliseconds.If the unit is going to be repeated in the individual entries for the column, it should be omitted from the column header.
On the other hand, if all of the entries are normalized to the same unit, the entries should not also include a unit suffix — especially not one for a unit of a different size!
The text was updated successfully, but these errors were encountered: