Skip to content
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

A more compact, colourful, output style. #215

Closed
tecosaur opened this issue May 26, 2021 · 9 comments · Fixed by #217
Closed

A more compact, colourful, output style. #215

tecosaur opened this issue May 26, 2021 · 9 comments · Fixed by #217

Comments

@tecosaur
Copy link
Collaborator

tecosaur commented May 26, 2021

Hi!

I find this tool great to use, but I can't help but think that the visual output could be polished a bit. One micro-benchmark tool that I think does a really nice job with this is hyperfine.

image

I think looking at what's done well by it could lead to a Benchmark output that's a bit easier to glance at 🙂

For example:

image

which I think is nicer than

image

@tecosaur
Copy link
Collaborator Author

If you'd like, I'd be happy to make a PR implementing this 🙂.

@tecosaur
Copy link
Collaborator Author

tecosaur commented May 26, 2021

Oh, another idea I've just had. Instead of a boxplot, one could use the unicode block characters to produce a histogram.

image

@tecosaur
Copy link
Collaborator Author

So, the above is a mock-up I made by echoing a string + shell-escape colour codes. But I liked the idea enough that I thought I'd code it up for my own beautification if nothing else. This is what I currently have:

image

@tecosaur
Copy link
Collaborator Author

A heuristic to determine if the frequency should be logged would be good I think. Please let me know if you have any good ideas for a check. For reference, the above without logging frequency looks like this:

image

which I think is far less informative.

@tecosaur
Copy link
Collaborator Author

Heuristic determined, I think I'll turn this into a PR shortly.

@Arkoniak
Copy link

Arkoniak commented Jun 8, 2021

Other than color issues, I think it is not correct to override the current @benchmark macro which output is well known and readable. I mean, it depends on a user and I wouldn't be happy to see this new design, sorry.

Maybe change it to @hyperfine for those, who wants to use this visualization? We already have @btime and @benchmark for different outputs, so it is acceptable I guess to add another one.

@tecosaur
Copy link
Collaborator Author

tecosaur commented Jun 8, 2021

I think it is not correct to override the current @benchmark macro which output is well known and readable.

I must admit I don't quite see your point here. My proposed replacement is perfectly readable, and I don't see the point of "well known" unless the way information is structured takes much getting used to --- and I don't think either the current or my proposed approaches do.

@Arkoniak
Copy link

Arkoniak commented Jun 8, 2021

My point is that all people are different and some prefer an existing version of @benchmark output to your proposal. It is not fair to force this design on them. At the very least, people should have a choice what version they want to use.

@ptoche
Copy link

ptoche commented Jun 11, 2021

IMHO the colors and other refinements should go into another package.

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 a pull request may close this issue.

3 participants