-
Notifications
You must be signed in to change notification settings - Fork 1.1k
bench_ecmult: add benchmark for ecmult_const_xonly #1668
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
bench_ecmult: add benchmark for ecmult_const_xonly #1668
Conversation
oh interesting to see the result! I'm getting something similar on my machine too. Looks like ~54% of ![]() also I ran the benchmarks in b62b7b7 -
|
@stratospher Just out of curiosity, how did you generate this nice graph? |
CLion has a 1 click option to generate flame graphs. 😊 |
ACK cc71265 |
cc71265
to
0544537
Compare
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.
ACK 0544537
9fab42525 Merge bitcoin-core/secp256k1#1668: bench_ecmult: add benchmark for ecmult_const_xonly 05445377f bench_ecmult: add benchmark for ecmult_const_xonly bb597b3d3 Merge bitcoin-core/secp256k1#1670: tests: update wycheproof files d73ed9947 tests: update wycheproof files 4187a4664 Merge bitcoin-core/secp256k1#1492: tests: Add Wycheproof ECDH vectors e266ba11a tests: Add Wycheproof ECDH vectors 13906b715 Merge bitcoin-core/secp256k1#1669: gitignore: Add Python cache files c1bcb0327 gitignore: Add Python cache files 70f149b9a Merge bitcoin-core/secp256k1#1662: bench: add ellswift to bench help output 6b3fe51fb bench: add ellswift to bench help output d84bb83e2 Merge bitcoin-core/secp256k1#1661: configure: Show exhaustive tests in summary 3f54ed8c1 Merge bitcoin-core/secp256k1#1659: include: remove WARN_UNUSED_RESULT for functions always returning 1 20b05c9d3 configure: Show exhaustive tests in summary e56716a3b Merge bitcoin-core/secp256k1#1660: ci: Fix exiting from ci.sh on error d87c3bc58 ci: Fix exiting from ci.sh on error 1b6e08153 include: remove WARN_UNUSED_RESULT for functions always returning 1 2abb35b03 Merge bitcoin-core/secp256k1#1657: tests: remove unused uncounting_illegal_callback_fn 51907fa91 tests: remove unused uncounting_illegal_callback_fn a7a511714 Merge bitcoin-core/secp256k1#1359: Fix symbol visibility issues, add test for it 13ed6f65d Merge bitcoin-core/secp256k1#1593: Remove deprecated `_ec_privkey_{negate,tweak_add,tweak_mul}` aliases from API d1478763a build: Drop no longer needed `-fvisibility=hidden` compiler option 8ed1d83d9 ci: Run `tools/symbol-check.py` 41d32ab2d test: Add `tools/symbol-check.py` 88548058b Introduce `SECP256K1_LOCAL_VAR` macro 03bbe8c61 Merge bitcoin-core/secp256k1#1655: gha: Print all *.log files, in a separate action 59860bcc2 gha: Print all *.log files, in a separate action 4ba1ba2af Merge bitcoin-core/secp256k1#1647: cmake: Adjust diagnostic flags for `clang-cl` abd25054a Merge bitcoin-core/secp256k1#1656: musig: Fix clearing of pubnonces 961ec25a8 musig: Fix clearing of pubnonces 318608238 Merge bitcoin-core/secp256k1#1614: Add _ge_set_all_gej and use it in musig for own public nonces 6c2a39daf Merge bitcoin-core/secp256k1#1639: Make static context const 37d2c60be Remove deprecated _ec_privkey_{negate,tweak_add,tweak_mul} aliases 432ac5770 Make static context const 1b1fc0934 Merge bitcoin-core/secp256k1#1642: Verify `compressed` argument in `secp256k1_eckey_pubkey_serialize` c0d9480fb Merge bitcoin-core/secp256k1#1654: use `EXIT_` constants over magic numbers for indicating program execution status 13d389629 CONTRIBUTING: mention that `EXIT_` codes should be used c85558172 test, bench, precompute_ecmult: use `EXIT_...` constants for `main` return values 965393fce examples: use `EXIT_...` constants for `main` return values 2e3bf1365 Merge bitcoin-core/secp256k1#1646: README: add instructions for verifying GPG signatures b682dbcf8 README: add instructions for verifying GPG signatures 00774d072 Merge bitcoin-core/secp256k1#1650: schnorrsig: clear out masked secret key in BIP-340 nonce function a82287fb8 schnorrsig: clear out masked secret key in BIP-340 nonce function 4c50d73dd ci: Add new "Windows (clang-cl)" job 84c0bd1f7 cmake: Adjust diagnostic flags for clang-cl f79f46c70 Merge bitcoin-core/secp256k1#1641: doc: Improve cmake instructions in README 2ac9f558c doc: Improve cmake instructions in README 182359476 Verify `compressed` argument in `secp256k1_eckey_pubkey_serialize` 64228a648 musig: Use _ge_set_all_gej for own public nonces 300aab1c0 tests: Improve _ge_set_all_gej(_var) tests 365f274ce group: Simplify secp256k1_ge_set_all_gej d3082ddea group: Add constant-time secp256k1_ge_set_all_gej git-subtree-dir: src/secp256k1 git-subtree-split: 9fab4252567661574cc9f6f97a057884f8129ff2
51d2ecd6e9 cmake: add a helper for linking into static libs 201b2b8f06 Merge bitcoin-core/secp256k1#1675: cmake: Bump minimum required CMake version to 3.22 3af71987a8 cmake: Bump minimum required CMake version to 3.22 92394476e9 Merge bitcoin-core/secp256k1#1673: Assert field magnitude at control-flow join 3a4f448cb4 Assert field magnitude at control-flow join 9fab425256 Merge bitcoin-core/secp256k1#1668: bench_ecmult: add benchmark for ecmult_const_xonly 05445377f4 bench_ecmult: add benchmark for ecmult_const_xonly bb597b3d39 Merge bitcoin-core/secp256k1#1670: tests: update wycheproof files d73ed99479 tests: update wycheproof files git-subtree-dir: src/secp256k1 git-subtree-split: 51d2ecd6e9f8ec1048d04fae34c2430c749d3bff
93bb76daa1 build: Add SECP256K1_FORCE_HIDDEN_VISIBILITY 89096c234d Merge bitcoin-core/secp256k1#1692: cmake: configure libsecp256k1.pc during install 7106dce6fd cmake: configure libsecp256k1.pc during install 29e73f4ba5 Merge bitcoin-core/secp256k1#1685: cmake: Emulate Libtool's behavior on FreeBSD 746e36b141 Merge bitcoin-core/secp256k1#1678: cmake: add a helper for linking into static libs a28c2ffa5c Merge bitcoin-core/secp256k1#1683: README: add link to musig example 2a9d374735 Merge bitcoin-core/secp256k1#1690: ci: Bump GCC snapshot major version to 16 add146e101 ci: Bump GCC snapshot major version to 16 145ae3e28d cmake: add a helper for linking into static libs 819210974b README: add link to musig example, generalize module enabling hint 95db29b144 Merge bitcoin-core/secp256k1#1679: cmake: Use `PUBLIC_HEADER` target property in installation logic 37dd422b5c cmake: Emulate Libtool's behavior on FreeBSD f24b838bed Merge bitcoin-core/secp256k1#1680: doc: Promote "Building with CMake" to standard procedure 3f31ac43e0 doc: Promote "Building with CMake" to standard procedure 6f67151ee2 cmake: Use `PUBLIC_HEADER` target property c32715b2a0 cmake, move-only: Move module option processing to `src/CMakeLists.txt` 201b2b8f06 Merge bitcoin-core/secp256k1#1675: cmake: Bump minimum required CMake version to 3.22 3af71987a8 cmake: Bump minimum required CMake version to 3.22 92394476e9 Merge bitcoin-core/secp256k1#1673: Assert field magnitude at control-flow join 3a4f448cb4 Assert field magnitude at control-flow join 9fab425256 Merge bitcoin-core/secp256k1#1668: bench_ecmult: add benchmark for ecmult_const_xonly 05445377f4 bench_ecmult: add benchmark for ecmult_const_xonly bb597b3d39 Merge bitcoin-core/secp256k1#1670: tests: update wycheproof files d73ed99479 tests: update wycheproof files git-subtree-dir: src/secp256k1 git-subtree-split: 93bb76daa1905974a22fab43f2675a53c85f000d
6264c3d093 docs: update README f825d34260 ci: enable silentpayments module b821a467e2 tests: add constant time tests b5b73bcd99 tests: add BIP-352 test vectors eabeedb752 silentpayments: add benchmarks for scanning 1de8b7e854 silentpayments: add examples/silentpayments.c ed3a44b10a silentpayments: receiving 3c9362dd6a silentpayments: recipient label support 70e20b7145 silentpayments: sending cf44324b5e build: add skeleton for new silentpayments (BIP352) module ad60ef7ea7 Merge bitcoin-core/secp256k1#1689: ci: Convert `arm64` Cirrus tasks to GHA jobs c498779096 Merge bitcoin-core/secp256k1#1687: cmake: support the use of launchers in ctest -S scripts 0dfe387dbe cmake: support the use of launchers in ctest -S scripts 89096c234d Merge bitcoin-core/secp256k1#1692: cmake: configure libsecp256k1.pc during install 7106dce6fd cmake: configure libsecp256k1.pc during install 29e73f4ba5 Merge bitcoin-core/secp256k1#1685: cmake: Emulate Libtool's behavior on FreeBSD 746e36b141 Merge bitcoin-core/secp256k1#1678: cmake: add a helper for linking into static libs a28c2ffa5c Merge bitcoin-core/secp256k1#1683: README: add link to musig example 2a9d374735 Merge bitcoin-core/secp256k1#1690: ci: Bump GCC snapshot major version to 16 add146e101 ci: Bump GCC snapshot major version to 16 004f57fcd8 ci: Move Valgrind build for `arm64` from Cirrus to GHA 5fafdfc30f ci: Move `gcc-snapshot` build for `arm64` from Cirrus to GHA e814b79a8b ci: Switch `arm64_debian` from QEMU to native `arm64` Docker image bcf77346b9 ci: Add `arm64` architecture to `docker_cache` job b77aae9226 ci: Rename Docker image tag to reflect architecture 145ae3e28d cmake: add a helper for linking into static libs 819210974b README: add link to musig example, generalize module enabling hint 95db29b144 Merge bitcoin-core/secp256k1#1679: cmake: Use `PUBLIC_HEADER` target property in installation logic 37dd422b5c cmake: Emulate Libtool's behavior on FreeBSD f24b838bed Merge bitcoin-core/secp256k1#1680: doc: Promote "Building with CMake" to standard procedure 3f31ac43e0 doc: Promote "Building with CMake" to standard procedure 6f67151ee2 cmake: Use `PUBLIC_HEADER` target property c32715b2a0 cmake, move-only: Move module option processing to `src/CMakeLists.txt` 201b2b8f06 Merge bitcoin-core/secp256k1#1675: cmake: Bump minimum required CMake version to 3.22 3af71987a8 cmake: Bump minimum required CMake version to 3.22 92394476e9 Merge bitcoin-core/secp256k1#1673: Assert field magnitude at control-flow join 3a4f448cb4 Assert field magnitude at control-flow join 9fab425256 Merge bitcoin-core/secp256k1#1668: bench_ecmult: add benchmark for ecmult_const_xonly 05445377f4 bench_ecmult: add benchmark for ecmult_const_xonly bb597b3d39 Merge bitcoin-core/secp256k1#1670: tests: update wycheproof files d73ed99479 tests: update wycheproof files 4187a46649 Merge bitcoin-core/secp256k1#1492: tests: Add Wycheproof ECDH vectors e266ba11ae tests: Add Wycheproof ECDH vectors 13906b7154 Merge bitcoin-core/secp256k1#1669: gitignore: Add Python cache files c1bcb03276 gitignore: Add Python cache files 70f149b9a1 Merge bitcoin-core/secp256k1#1662: bench: add ellswift to bench help output 6b3fe51fb6 bench: add ellswift to bench help output d84bb83e26 Merge bitcoin-core/secp256k1#1661: configure: Show exhaustive tests in summary 3f54ed8c1b Merge bitcoin-core/secp256k1#1659: include: remove WARN_UNUSED_RESULT for functions always returning 1 20b05c9d3f configure: Show exhaustive tests in summary e56716a3bc Merge bitcoin-core/secp256k1#1660: ci: Fix exiting from ci.sh on error d87c3bc58f ci: Fix exiting from ci.sh on error 1b6e081538 include: remove WARN_UNUSED_RESULT for functions always returning 1 2abb35b034 Merge bitcoin-core/secp256k1#1657: tests: remove unused uncounting_illegal_callback_fn 51907fa918 tests: remove unused uncounting_illegal_callback_fn a7a5117144 Merge bitcoin-core/secp256k1#1359: Fix symbol visibility issues, add test for it 13ed6f65dc Merge bitcoin-core/secp256k1#1593: Remove deprecated `_ec_privkey_{negate,tweak_add,tweak_mul}` aliases from API d1478763a5 build: Drop no longer needed `-fvisibility=hidden` compiler option 8ed1d83d92 ci: Run `tools/symbol-check.py` 41d32ab2de test: Add `tools/symbol-check.py` 88548058b3 Introduce `SECP256K1_LOCAL_VAR` macro 03bbe8c615 Merge bitcoin-core/secp256k1#1655: gha: Print all *.log files, in a separate action 59860bcc24 gha: Print all *.log files, in a separate action 37d2c60bec Remove deprecated _ec_privkey_{negate,tweak_add,tweak_mul} aliases REVERT: c0db6509bd docs: update README REVERT: 8339232b7e ci: enable silentpayments module REVERT: 635745fc3a tests: add constant time tests REVERT: b1de2ee2f7 tests: add BIP-352 test vectors REVERT: aea372837f silentpayments: add benchmarks for scanning REVERT: 1ec7857aed silentpayments: add examples/silentpayments.c REVERT: c9bec084eb silentpayments: receiving REVERT: 28fd17d7c4 silentpayments: recipient label support REVERT: 065e8b7793 silentpayments: sending REVERT: a6d8b11754 build: add skeleton for new silentpayments (BIP352) module REVERT: 6274359346 bench: add ellswift to bench help output REVERT: 0258186573 configure: Show exhaustive tests in summary REVERT: 53b578d10b include: remove WARN_UNUSED_RESULT for functions always returning 1 REVERT: f75c985604 ci: Fix exiting from ci.sh on error REVERT: 947761b842 tests: remove unused uncounting_illegal_callback_fn REVERT: 5d01f375c6 build: Drop no longer needed `-fvisibility=hidden` compiler option REVERT: dbf1e95d2a ci: Run `tools/symbol-check.py` REVERT: 8174c88f47 test: Add `tools/symbol-check.py` REVERT: 8a287f9a32 Introduce `SECP256K1_LOCAL_VAR` macro REVERT: 7106544a16 Remove deprecated _ec_privkey_{negate,tweak_add,tweak_mul} aliases REVERT: 1e2da62eff gha: Print all *.log files, in a separate action git-subtree-dir: src/secp256k1 git-subtree-split: 6264c3d0939f2ab11ba8c92f3cb521f9c89c8596
6264c3d093 docs: update README f825d34260 ci: enable silentpayments module b821a467e2 tests: add constant time tests b5b73bcd99 tests: add BIP-352 test vectors eabeedb752 silentpayments: add benchmarks for scanning 1de8b7e854 silentpayments: add examples/silentpayments.c ed3a44b10a silentpayments: receiving 3c9362dd6a silentpayments: recipient label support 70e20b7145 silentpayments: sending cf44324b5e build: add skeleton for new silentpayments (BIP352) module ad60ef7ea7 Merge bitcoin-core/secp256k1#1689: ci: Convert `arm64` Cirrus tasks to GHA jobs c498779096 Merge bitcoin-core/secp256k1#1687: cmake: support the use of launchers in ctest -S scripts 0dfe387dbe cmake: support the use of launchers in ctest -S scripts 89096c234d Merge bitcoin-core/secp256k1#1692: cmake: configure libsecp256k1.pc during install 7106dce6fd cmake: configure libsecp256k1.pc during install 29e73f4ba5 Merge bitcoin-core/secp256k1#1685: cmake: Emulate Libtool's behavior on FreeBSD 746e36b141 Merge bitcoin-core/secp256k1#1678: cmake: add a helper for linking into static libs a28c2ffa5c Merge bitcoin-core/secp256k1#1683: README: add link to musig example 2a9d374735 Merge bitcoin-core/secp256k1#1690: ci: Bump GCC snapshot major version to 16 add146e101 ci: Bump GCC snapshot major version to 16 004f57fcd8 ci: Move Valgrind build for `arm64` from Cirrus to GHA 5fafdfc30f ci: Move `gcc-snapshot` build for `arm64` from Cirrus to GHA e814b79a8b ci: Switch `arm64_debian` from QEMU to native `arm64` Docker image bcf77346b9 ci: Add `arm64` architecture to `docker_cache` job b77aae9226 ci: Rename Docker image tag to reflect architecture 145ae3e28d cmake: add a helper for linking into static libs 819210974b README: add link to musig example, generalize module enabling hint 95db29b144 Merge bitcoin-core/secp256k1#1679: cmake: Use `PUBLIC_HEADER` target property in installation logic 37dd422b5c cmake: Emulate Libtool's behavior on FreeBSD f24b838bed Merge bitcoin-core/secp256k1#1680: doc: Promote "Building with CMake" to standard procedure 3f31ac43e0 doc: Promote "Building with CMake" to standard procedure 6f67151ee2 cmake: Use `PUBLIC_HEADER` target property c32715b2a0 cmake, move-only: Move module option processing to `src/CMakeLists.txt` 201b2b8f06 Merge bitcoin-core/secp256k1#1675: cmake: Bump minimum required CMake version to 3.22 3af71987a8 cmake: Bump minimum required CMake version to 3.22 92394476e9 Merge bitcoin-core/secp256k1#1673: Assert field magnitude at control-flow join 3a4f448cb4 Assert field magnitude at control-flow join 9fab425256 Merge bitcoin-core/secp256k1#1668: bench_ecmult: add benchmark for ecmult_const_xonly 05445377f4 bench_ecmult: add benchmark for ecmult_const_xonly bb597b3d39 Merge bitcoin-core/secp256k1#1670: tests: update wycheproof files d73ed99479 tests: update wycheproof files git-subtree-dir: src/secp256k1 git-subtree-split: 6264c3d0939f2ab11ba8c92f3cb521f9c89c8596
9e85256bbe docs: update README 4b1fb2c186 ci: enable silentpayments module de508a78ac tests: add constant time tests 45427dd4d7 tests: add BIP-352 test vectors 6975614517 silentpayments: add benchmarks for scanning a9af9ebf35 silentpayments: add examples/silentpayments.c b06254b6c7 silentpayments: receiving 3c9362dd6a silentpayments: recipient label support 70e20b7145 silentpayments: sending cf44324b5e build: add skeleton for new silentpayments (BIP352) module ad60ef7ea7 Merge bitcoin-core/secp256k1#1689: ci: Convert `arm64` Cirrus tasks to GHA jobs c498779096 Merge bitcoin-core/secp256k1#1687: cmake: support the use of launchers in ctest -S scripts 0dfe387dbe cmake: support the use of launchers in ctest -S scripts 89096c234d Merge bitcoin-core/secp256k1#1692: cmake: configure libsecp256k1.pc during install 7106dce6fd cmake: configure libsecp256k1.pc during install 29e73f4ba5 Merge bitcoin-core/secp256k1#1685: cmake: Emulate Libtool's behavior on FreeBSD 746e36b141 Merge bitcoin-core/secp256k1#1678: cmake: add a helper for linking into static libs a28c2ffa5c Merge bitcoin-core/secp256k1#1683: README: add link to musig example 2a9d374735 Merge bitcoin-core/secp256k1#1690: ci: Bump GCC snapshot major version to 16 add146e101 ci: Bump GCC snapshot major version to 16 004f57fcd8 ci: Move Valgrind build for `arm64` from Cirrus to GHA 5fafdfc30f ci: Move `gcc-snapshot` build for `arm64` from Cirrus to GHA e814b79a8b ci: Switch `arm64_debian` from QEMU to native `arm64` Docker image bcf77346b9 ci: Add `arm64` architecture to `docker_cache` job b77aae9226 ci: Rename Docker image tag to reflect architecture 145ae3e28d cmake: add a helper for linking into static libs 819210974b README: add link to musig example, generalize module enabling hint 95db29b144 Merge bitcoin-core/secp256k1#1679: cmake: Use `PUBLIC_HEADER` target property in installation logic 37dd422b5c cmake: Emulate Libtool's behavior on FreeBSD f24b838bed Merge bitcoin-core/secp256k1#1680: doc: Promote "Building with CMake" to standard procedure 3f31ac43e0 doc: Promote "Building with CMake" to standard procedure 6f67151ee2 cmake: Use `PUBLIC_HEADER` target property c32715b2a0 cmake, move-only: Move module option processing to `src/CMakeLists.txt` 201b2b8f06 Merge bitcoin-core/secp256k1#1675: cmake: Bump minimum required CMake version to 3.22 3af71987a8 cmake: Bump minimum required CMake version to 3.22 92394476e9 Merge bitcoin-core/secp256k1#1673: Assert field magnitude at control-flow join 3a4f448cb4 Assert field magnitude at control-flow join 9fab425256 Merge bitcoin-core/secp256k1#1668: bench_ecmult: add benchmark for ecmult_const_xonly 05445377f4 bench_ecmult: add benchmark for ecmult_const_xonly bb597b3d39 Merge bitcoin-core/secp256k1#1670: tests: update wycheproof files d73ed99479 tests: update wycheproof files git-subtree-dir: src/secp256k1 git-subtree-split: 9e85256bbe527bf084222ee08dade9ea497d5c99
In the course of trying to understand the x-only ECDH benchmark results in the silent payments PR (#1519 (comment)), I noticed that we don't have a benchmark yet for
ecmult_const_xonly
, so I added one.Results on my machine:
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.