-
Notifications
You must be signed in to change notification settings - Fork 10
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
Store computed metrics in BenchmarkResult and move report/pretty-print functionality into BenchmarkResult #38
Store computed metrics in BenchmarkResult and move report/pretty-print functionality into BenchmarkResult #38
Conversation
report/pretty-print functionality into BenchmarkResult - Update examples - Add total-time metric and display it when pretty-printing
This can be considered (1/3) of the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent progress. The printing logic is quite lengthy and involves multiple steps. It might be beneficial to break this down into smaller, reusable functions for improved readability and maintainability. What do you think?
I agree with this. In the future we may add functionality for the user to choose which metrics to display, having split everything up into individual functions makes that easier. I'll address the feedback as soon as I have some time. |
- Split BenchmarkResult.prettyPrint into individual functions with respect to each metric to be pretty-printed - Propagate choice of writer for pretty-print functions within BenchmarkResult - Add a buffered std-out writer as a field of BenchmarkResults. We use this for all formatted printing-operations within BenchmarkResults. - Make use of BenchmarkResults.getColor to color-code the total-time metric in BenchmarkResults.prettyPrint - Add various doc-comments
821e49f
to
3313c40
Compare
Note: In 3313c40 I make use of Edit: By the way if you don't agree with letting the caller select the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of small things, but really cool stuff. 👍
- Rename BenchmarkResult.percs to BenchmarkResult.percentiles
1389ebf
into
hendriknielaender:main
Also added a total-time metric which represents how long the entire benchmark (all runs included) took to run. This is displayed when reporting/pretty printing. Also notably; we no longer print the result of a run after completion, the user must explicitly call
prettyPrint
on theBenchmarkResults
instance. Updated examples to reflect this.