bench_ecmult: improve clarity of output #999

jonasnick commented at 12:48 PM on October 22, 2021: contributor

Previously "ecmult{,_multi} xg" meant multiplication with (x - 1) random points and base point G. Now

xP means multiplication with x random points and
xP & G means multiplication with x random points and G

real-or-random commented at 3:10 PM on October 22, 2021: contributor

Concept ACK

Hm, I only notice now that the sed command we provide is not compatible with the output of this particular benchmark binary because it strips all spaces (with and without this PR). I assumed in #989 that the benchmark "names" never have spaces. Maybe we can rename it to something like ecmult_Xp_g or so?

siv2r commented at 1:13 AM on October 23, 2021: contributor

Concept ACK

Maybe we can rename it to something like ecmult_Xp_g or so?

I agree with this point. This will make the benchmark naming more uniform and compatible with sed.

Currently, the ecmult 1, ecmult 1g look like this in the CSV output.

Benchmark,Min(us),Avg(us),Max(us)
ecmult1,107.0,111.0,114.0
ecmult1g,76.3,79.9,83.6
ecmult2g,62.2,62.6,63.2
ecmult_multi1g,75.2,76.2,79.9
ecmult_multi2g,91.6,92.0,92.8

Rogelio165 approved

bench_ecmult: improve clarity of output

Previously "ecmult{,_multi} xg" meant multiplication with (x - 1) random points
and base point G. Now
- ecmult_{,multi_}xp means multiplication with x random points and
- ecmult_{,multi_}xp_g means multiplication with x random points and G

96b1ad2ea9

bench: don't return 1 in have_flag() if argc = 1

This makes the semantic of have_flag more clear and fixes a bug
that was introduced in

2fe1b50df16c9f41ea77b151634d734b930eeddd
Add ecmult_gen, ecmult_const and ecmult to benchmark

where the behavior introduced by this commit was already assumed. If
bench_ecmult was called without arguments, have_flag("simple") returned 1 and no
scratch space was allocated which led to very wrong output.

23e2f66726

jonasnick force-pushed on Oct 24, 2021

jonasnick commented at 7:49 PM on October 24, 2021: contributor

Maybe we can rename it to something like ecmult_Xp_g

Good point, done. Also fixed a bug that led to the "simple" ecmult_multi algorithm being when bench_ecmult was called without arguments (see commit message).

siv2r commented at 10:36 PM on October 24, 2021: contributor

tACK 23e2f66

The benchmark functions of bench_ecmult are appropriately renamed. The output of bench_emult is:

Benchmark                     ,    Min(us)    ,    Avg(us)    ,    Max(us)    

ecmult_gen                    ,    42.6       ,    43.0       ,    44.1    
ecmult_const                  ,    87.5       ,    87.7       ,    88.7    
ecmult_1p                     ,    65.4       ,    65.6       ,    65.9    
ecmult_0p_g                   ,    45.8       ,    46.4       ,    46.8    
ecmult_1p_g                   ,    38.3       ,    38.8       ,    39.5    
ecmult_multi_0p_g             ,    45.9       ,    46.4       ,    47.6    
ecmult_multi_1p_g             ,    38.3       ,    38.6       ,    39.3    
ecmult_multi_2p_g             ,    35.6       ,    35.7       ,    35.8    
ecmult_multi_3p_g             ,    34.3       ,    34.4       ,    34.5

real-or-random approved

real-or-random commented at 8:54 AM on October 25, 2021: contributor

ACK 23e2f66726f930ac01d5075106aa16a4073442b4

real-or-random merged this on Oct 25, 2021

real-or-random closed this on Oct 25, 2021

sipa referenced this in commit f727914d7e on Oct 28, 2021

sipa referenced this in commit 440f7ec80e on Oct 31, 2021

sipa referenced this in commit d057eae556 on Dec 2, 2021

fanquake referenced this in commit c4a1e09a8c on Dec 3, 2021

sipa referenced this in commit 86dbc4d075 on Dec 15, 2021

jonasnick cross-referenced this on Jan 2, 2022 from issue Sync Upstream by jonasnick

real-or-random referenced this in commit 21e2d65b79 on Jan 5, 2022

gwillen referenced this in commit 35d6112a72 on May 25, 2022

janus referenced this in commit 879a9a27b9 on Jul 10, 2022

patricklodder referenced this in commit 21badcf9d2 on Jul 25, 2022

patricklodder referenced this in commit 03002a9013 on Jul 28, 2022

backpacker69 referenced this in commit 77186f4a04 on Jan 18, 2023

str4d referenced this in commit 6de4698bf9 on Apr 21, 2023

vmta referenced this in commit e1120c94a1 on Jun 4, 2023

vmta referenced this in commit 8f03457eed on Jul 1, 2023