bench_ecmult: add benchmark for ecmult_const_xonly #1668

pull theStack wants to merge 1 commits into bitcoin-core:master from theStack:add-ecmult_const_xonly-benchmark changing 1 files +31 −0

theStack commented at 12:41 AM on March 27, 2025: contributor
In the course of trying to understand the x-only ECDH benchmark results in the silent payments PR (https://github.com/bitcoin-core/secp256k1/pull/1519#discussion_r2006144436), I noticed that we don't have a benchmark yet for ecmult_const_xonly, so I added one.

Results on my machine:
```
$ ./build/bin/bench_ecmult
Benchmark                     ,    Min(us)    ,    Avg(us)    ,    Max(us)

ecmult_gen                    ,    12.3       ,    12.6       ,    13.2
ecmult_const                  ,    27.2       ,    28.6       ,    30.9
ecmult_const_xonly            ,    29.3       ,    30.4       ,    32.6

...
```
I was a bit surprised to see that the x-only variant is slower than the regular ecmult_const function, but on the other hand the former allows to skip pubkey deserialization (i.e. determining y with a square root calculation, which is rather expensive), so that has to be taken account for a fair comparison in use-cases like ECDH (see also benchmark commit in #1198). Having an isolated ecmult benchmark for it still seems to make sense, imho. Note that the teardown function is a bit hacky; since we don't have the parity of the x-only point multiplication results, the full point multiplication ones are calculated and the x coordinates are compared, and bench_ecmult_teardown_helper isn't used.
stratospher commented at 5:43 PM on March 27, 2025: contributor
oh interesting to see the result! I'm getting something similar on my machine too. Looks like ~54% of ecmult_const_xonly's execution time is spent in ecmult_const (inside the ecmult_const_xonly).

<img width="1327" alt="with d = NULL" src="https://github.com/user-attachments/assets/96218d31-e829-460c-b2d5-6261bcd44457" />

also I ran the benchmarks in https://github.com/bitcoin-core/secp256k1/pull/1198/commits/b62b7b72fa6e20eee796750bde47054a9d00258 -bench_ecdh was few microseconds slower/bench_ecdh_xonly was few microseconds faster. But when I remove secp256k1_ec_pubkey_parse from bench_ecdh, I'm seeing a similar performance trend in the ecdh, ecdh_xonly benchmark - bench_ecdh_xonly is a few microseconds slower/bench_ecdh is few microseconds faster.
```
Benchmark                     ,    Min(us)    ,    Avg(us)    ,    Max(us)

ecdh                          ,    23.5       ,    24.1       ,    26.0    
ecdh_xonly                    ,    25.0       ,    25.1       ,    25.2  
```
real-or-random commented at 1:39 PM on March 31, 2025: contributor

@stratospher Just out of curiosity, how did you generate this nice graph?
stratospher commented at 1:54 PM on March 31, 2025: contributor

CLion has a 1 click option to generate flame graphs. 😊
sipa commented at 9:32 PM on May 12, 2025: contributor

ACK cc712658a5bd439a6fafcc2bb8d918d7e571e3d4

in src/bench_ecmult.c:136 in cc712658a5 outdated

 130 | +
 131 | +    for (i = 0; i < iters; ++i) {
 132 | +        const secp256k1_ge* pubkey = &data->pubkeys[(data->offset1+i) % POINTS];
 133 | +        const secp256k1_scalar* scalar = &data->scalars[(data->offset2+i) % POINTS];
 134 | +        secp256k1_ecmult_const_xonly(&data->output_xonly[i], &pubkey->x, NULL, scalar, 1);
 135 | +    }

jonasnick commented at 1:24 PM on May 14, 2025:

nit: the argument is critical to performance and this documents it a bit better

    for (i = 0; i < iters; ++i) {
        const secp256k1_ge* pubkey = &data->pubkeys[(data->offset1+i) % POINTS];
        const secp256k1_scalar* scalar = &data->scalars[(data->offset2+i) % POINTS];
        int known_on_curve = 1;
        secp256k1_ecmult_const_xonly(&data->output_xonly[i], &pubkey->x, NULL, scalar, known_on_curve);
    }

theStack commented at 3:46 PM on May 14, 2025:

Thanks, done.

in src/bench_ecmult.c:150 in cc712658a5 outdated

 144 | +        const secp256k1_ge* pubkey = &data->pubkeys[(data->offset1+i) % POINTS];
 145 | +        const secp256k1_scalar* scalar = &data->scalars[(data->offset2+i) % POINTS];
 146 | +        secp256k1_gej expected_gej;
 147 | +        secp256k1_ecmult_const(&expected_gej, pubkey, scalar);
 148 | +        CHECK(secp256k1_gej_eq_x_var(&data->output_xonly[i], &expected_gej));
 149 | +    }

jonasnick commented at 1:25 PM on May 14, 2025:

Is there a reason you don't use the faster ecmult function, like so?

    for (i = 0; i < iters; ++i) {
        const secp256k1_ge* pubkey = &data->pubkeys[(data->offset1+i) % POINTS];
        const secp256k1_scalar* scalar = &data->scalars[(data->offset2+i) % POINTS];
        secp256k1_gej pubkey_gej;
        secp256k1_gej expected_gej;
        secp256k1_gej_set_ge(&pubkey_gej, pubkey);
        secp256k1_ecmult(&expected_gej, &pubkey_gej, scalar, NULL);
        CHECK(secp256k1_gej_eq_x_var(&data->output_xonly[i], &expected_gej));
    }

theStack commented at 3:49 PM on May 14, 2025:

I don't remember, likely I thought that directly comparing with the non-x-only variant of the same function is just a bit more consistent. Changed it to compare with the faster ecmult (turns out the _gej_set_ge call isn't even needed as the pubkeys are also available in jacobi format in the bench_data struct, so it doesn't need more lines of code 👍 ).

jonasnick commented at 7:56 PM on May 14, 2025:

Ah, nice find

bench_ecmult: add benchmark for ecmult_const_xonly 05445377f4
theStack force-pushed on May 14, 2025
jonasnick approved
jonasnick commented at 7:56 PM on May 14, 2025: contributor

ACK 05445377f4a1d5837b9e1616c8f1abfdf72ad9c2
jonasnick merged this on May 14, 2025
jonasnick closed this on May 14, 2025
theStack deleted the branch on May 14, 2025

Contributors

Linked (view graph)

#1198 Add x-only ECDH support to ecdh module #1519 Add BIP352 `silentpayments` module