Skip to content

feat: improve the latency and throughput formatting in the result table - #233

Merged
jerome-benoit merged 9 commits into
tinylibs:mainfrom
pallosp:main
Jan 29, 2025
Merged

feat: improve the latency and throughput formatting in the result table#233
jerome-benoit merged 9 commits into
tinylibs:mainfrom
pallosp:main

Conversation

@pallosp

@pallosp pallosp commented Jan 28, 2025

Copy link
Copy Markdown
Contributor

Make the result table a bit more compact, as per the discussion under issue #231

  • Reduce the number of latency and throughput digits, because the least significant ones were just noise.
  • Drop the ± part from the medians. They indicated the distance between the two middle elements when there was even number of them, which isn't particularly useful.
  • Output the medians as numbers to get rid of the quotes.

Example result table for simple benchmark gc:

┌─────────┬───────────────┬──────────────────────┬─────────────────────┬────────────────────────────┬───────────────────────────┬─────────┐
│ (index) │ Task name     │ Latency average (ns) │ Latency median (ns) │ Throughput average (ops/s) │ Throughput median (ops/s) │ Samples │
├─────────┼───────────────┼──────────────────────┼─────────────────────┼────────────────────────────┼───────────────────────────┼─────────┤
│ 0       │ 'faster task' │ '3578.7 ± 2.83%'     │ 3125                │ '314227 ± 0.18%'           │ 320000                    │ 27944   │
│ 1       │ 'slower task' │ '1164166 ± 3.01%'    │ 1192251             │ '1321 ± 67.29%'            │ 839                       │ 86      │
└─────────┴───────────────┴──────────────────────┴─────────────────────┴────────────────────────────┴───────────────────────────┴─────────┘

Comment thread src/bench.ts
'Latency average (ns)': `${mToNs(task.result.latency.mean).toFixed(2)} \xb1 ${task.result.latency.rme.toFixed(2)}%`,
'Latency average (ns)': `${formatNumber(mToNs(task.result.latency.mean), 5, 2)} \xb1 ${task.result.latency.rme.toFixed(2)}%`,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
'Latency median (ns)': `${mToNs(task.result.latency.p50!).toFixed(2)}${Number.parseFloat(mToNs(task.result.latency.mad!).toFixed(2)) > 0 ? ` \xb1 ${mToNs(task.result.latency.mad!).toFixed(2)}` : ''}`,

@jerome-benoit jerome-benoit Jan 28, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the threshold condition here : when the median absolute deviation is upper zero, it's a sign of statistical significance issue during the benchmark and a valuable information.

You can use the same helper to display it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether I get correctly what the absolute deviation is supposed to measure.

So far I've only seen it in the table when the number of samples was even. If the median for even number of samples is problematic, shouldn't the task be executed one more time instead of reporting a problem?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's totally unrelated to the number of samples. It's a robust statistical indicator of the statistical significance of the median value. And the only one for median. Like the standard deviation (or the absolute mean deviation) is for the average.

Adding a threshold to it is only way to reduce the information to display.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. After some debugging I understood what's going on. There is a bug in absoluteDeviation that makes mad exactly zero as opposed to a small positive number in most cases. I filed #234.

Comment thread src/bench.ts
: (convert?.(task) ?? {
'Task name': task.name,
'Latency average (ns)': `${mToNs(task.result.latency.mean).toFixed(2)} \xb1 ${task.result.latency.rme.toFixed(2)}%`,
'Latency average (ns)': `${formatNumber(mToNs(task.result.latency.mean), 5, 2)} \xb1 ${task.result.latency.rme.toFixed(2)}%`,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can also use the formaNumber helper for the standard deviation.

Displaying number only in the column would be better, but I do not think it's possible because of string only characters usage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For percentages less than 1000% the helper returns the same string as toFixed(2). I slightly prefer keeping toFixed(2) because of its simpler semantics.

@pkg-pr-new

pkg-pr-new Bot commented Jan 28, 2025

Copy link
Copy Markdown

Open in Stackblitz

npm i https://pkg.pr.new/tinylibs/tinybench@233

commit: 392af32

Comment thread src/bench.ts Outdated
Comment thread src/bench.ts Outdated
Comment thread test/index.test.ts Outdated
Comment thread test/index.test.ts Outdated
Comment thread test/index.test.ts Outdated
Comment thread test/index.test.ts Outdated
@jerome-benoit
jerome-benoit merged commit 366cf99 into tinylibs:main Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants