simplify the std bench
improvement candidate
#865
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@amtoine gave me valuable feedback about the PR of
std bench
improvement CANDIDATE intostdlib-candidates
.I understood and respected his reasoning about his position. Yet, I still believe that users of
std bench
will benefit from my proposal.I incorporated some of @amtoine advice in this PR.
I removed the
bench --units
option as I now believe it is better to encourage users to use core functionality for formatting duration (which I had not thought of initially) and to avoid multiplying--options
.Also, I removed the option of
--sign-digits
and just hard-coded the precision of conversion to the fourth significant digit (which will give a maximum relative error of 0.05%, which I still think is unnecessarily precise).To explain my motivation, I added some context from our previous conversation:
And I add here that if Nushell adds the setting for resetting insignificant digits from displaying, those conversions could be removed for the better. Yet, we don't have such a setting yet, but we already use bench and use it quite often. So, I propose this usability improvement.
In my defense, I would add that the existing
--pretty
option will only benefit from the proposed changes (while it can't benefit from| format duration ms
).Finally, I kept the
--list-timings
option because I strongly believe that users much more often will not need expanded 50 lines of timing results on their screen (which they can get rid of by adding| reject time
in the second execution of thebench
command - but I would like to avoid this second execution).I won't be hurt if my proposed changes aren't accepted and applied to mainstream. Yet, I feel like my initial PR is still valuable and will benefit from my current PR's additions.