From 308d858c4f9c7dff1d0b02856301c38b67e7ee18 Mon Sep 17 00:00:00 2001 From: Maxim Uvarov Date: Mon, 3 Jun 2024 21:35:33 +0800 Subject: [PATCH] simplify the `std bench` improvement candidate (#865) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @amtoine gave me valuable feedback about the [PR of `std bench` improvement](https://github.com/nushell/nu_scripts/pull/859) CANDIDATE into `stdlib-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`. ``` bench -n 10 {sleep 0.1sec} | format duration ms mean min max stddev # or bench -n 10 {sleep 0.1sec} | format duration ms ...($in | columns) ``` 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: >> Maybe you would agree with me that this representation is very wordy and unnecessary, even for a professional? >> `1sec 333ms 333ยตs 333ns` > maybe, but then i would argue the issue comes from the formatting nushell does on durations, not from std bench, it's just that Nushell will show every part of the duration ๐Ÿ˜• 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`). ```diff > bench {sleep (10sec / 3)} --rounds 2 --pretty - 3sec 335ms 974ยตs 264ns +/- 1ms 108ยตs 748ns + 3sec 335ms +/- 1ms 108ยตs ``` 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 the `bench` 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. --- stdlib-candidate/std-rfc/bench.nu | 30 +++++++----------------------- stdlib-candidate/tests/bench.nu | 10 ---------- 2 files changed, 7 insertions(+), 33 deletions(-) diff --git a/stdlib-candidate/std-rfc/bench.nu b/stdlib-candidate/std-rfc/bench.nu index 3c822ef..aa38956 100644 --- a/stdlib-candidate/std-rfc/bench.nu +++ b/stdlib-candidate/std-rfc/bench.nu @@ -5,17 +5,13 @@ use ./math # convert an integer amount of nanoseconds to a real duration def "from ns" [ - --units: string # units to convert duration to (min, sec, ms, ยตs, ns) - --sign-digits: int # a number of first non-zero digits to keep (default 4; set 0 to disable rounding) + --sign-digits: int = 4 # a number of first non-zero digits to keep (default 4; set 0 to disable rounding) ] { if $sign_digits == 0 {} else { math significant-digits $sign_digits } | $"($in)ns" | into duration - | if $units != null { - format duration $units - } else {} } # run a piece of `nushell` code multiple times and measure the time of execution. @@ -33,7 +29,6 @@ def "from ns" [ # # > **Note** # > `std bench --pretty` will return a `string`. -# > the `--units` option will convert all durations to strings # # # Examples # measure the performance of simple addition @@ -46,18 +41,9 @@ def "from ns" [ # โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”ดโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ # # get a pretty benchmark report -# > std bench {1 + 2} --pretty +# > bench {1 + 2} --pretty # 922ns +/- 2ยตs 40ns # -# format results as `ms` -# > bench {sleep 0.1sec; 1 + 2} --units ms --rounds 5 -# โ•ญโ”€โ”€โ”€โ”€โ”€โ”€โ”ฌโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ -# โ”‚ mean โ”‚ 104.90 ms โ”‚ -# โ”‚ min โ”‚ 103.60 ms โ”‚ -# โ”‚ max โ”‚ 105.90 ms โ”‚ -# โ”‚ std โ”‚ 0.75 ms โ”‚ -# โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”ดโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฏ -# # measure the performance of simple addition with 1ms delay and output each timing # > bench {sleep 1ms; 1 + 2} --rounds 2 --list-timings | table -e # โ•ญโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ฌโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ•ฎ @@ -75,9 +61,7 @@ export def main [ --rounds (-n): int = 50 # the number of benchmark rounds (hopefully the more rounds the less variance) --verbose (-v) # be more verbose (namely prints the progress) --pretty # shows the results in human-readable format: " +/- " - --units: string # units to convert duration to (min, sec, ms, ยตs, ns) --list-timings # list all rounds' timings in a `times` field - --sign-digits: int = 4 # a number of first non-zero digits to keep (default 4; set 0 to disable rounding) ] { let times = seq 1 $rounds | each {|i| @@ -88,16 +72,16 @@ export def main [ if $verbose { print $"($rounds) / ($rounds)" } { - mean: ($times | math avg | from ns --units $units --sign-digits=$sign_digits) - min: ($times | math min | from ns --units $units --sign-digits=$sign_digits) - max: ($times | math max | from ns --units $units --sign-digits=$sign_digits) - std: ($times | math stddev | from ns --units $units --sign-digits=$sign_digits) + mean: ($times | math avg | from ns --sign-digits 4) + min: ($times | math min | from ns --sign-digits 4) + max: ($times | math max | from ns --sign-digits 4) + std: ($times | math stddev | from ns --sign-digits 4) } | if $pretty { $"($in.mean) +/- ($in.std)" } else { if $list_timings { - merge { times: ($times | each { from ns --units $units --sign-digits 0 }) } + merge { times: ($times | each { from ns --sign-digits 0 }) } } else {} } } diff --git a/stdlib-candidate/tests/bench.nu b/stdlib-candidate/tests/bench.nu index 0e2c41f..815b6af 100644 --- a/stdlib-candidate/tests/bench.nu +++ b/stdlib-candidate/tests/bench.nu @@ -1,11 +1,6 @@ use std assert use ../std-rfc bench -export def "test bench-units" [] { - let $test = bench {sleep 0.001sec; 1 + 2} --units ns --rounds 2 | get mean - assert str contains $test " ns" -} - export def "test bench-timings" [] { let $test = bench {1 + 2} --rounds 3 --list-timings | get times | length assert equal $test 3 @@ -15,8 +10,3 @@ export def "test bench-pretty" [] { let $test = (bench {1 + 2} --rounds 3 --pretty) =~ '\d.* \+/- \d' assert equal $test true } - -export def "test bench-sign-digits" [] { - let $test = bench {sleep 1ms} --sign-digits 1 --rounds 5 | get min - assert equal $test 1ms -}