From 28f273d7feb888066ad8f521c910d57250a72c5f Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 26 Oct 2025 00:39:02 +0100 Subject: [PATCH 1/4] cmake: Add `FindClockGettime` module --- cmake/FindClockGettime.cmake | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 cmake/FindClockGettime.cmake diff --git a/cmake/FindClockGettime.cmake b/cmake/FindClockGettime.cmake new file mode 100644 index 0000000000..f5101c7165 --- /dev/null +++ b/cmake/FindClockGettime.cmake @@ -0,0 +1,47 @@ +#[=======================================================================[ +FindClockGettime +---------------- + +Finds the clock_gettime() POSIX function on the system. + +Imported Targets +^^^^^^^^^^^^^^^^ + +This module provides the following Imported Targets: + +POSIX::clock_gettime + Target encapsulating the clock_gettime() usage requirements, available + only if clock_gettime() is found. + +#]=======================================================================] + +include(CheckSymbolExists) +include(CMakePushCheckState) + +cmake_push_check_state(RESET) + +set(CMAKE_REQUIRED_DEFINITIONS -D_POSIX_C_SOURCE=199309L) +check_symbol_exists(clock_gettime "time.h" CLOCK_GETTIME_IS_BUILT_IN) +set(${CMAKE_FIND_PACKAGE_NAME}_FOUND ${CLOCK_GETTIME_IS_BUILT_IN}) + +if(NOT ${CMAKE_FIND_PACKAGE_NAME}_FOUND) + set(CMAKE_REQUIRED_LIBRARIES rt) + check_symbol_exists(clock_gettime "time.h" CLOCK_GETTIME_NEEDS_LINK_TO_LIBRT) + set(${CMAKE_FIND_PACKAGE_NAME}_FOUND ${CLOCK_GETTIME_NEEDS_LINK_TO_LIBRT}) +endif() + +if(${CMAKE_FIND_PACKAGE_NAME}_FOUND) + if(NOT TARGET POSIX::clock_gettime) + add_library(POSIX::clock_gettime INTERFACE IMPORTED) + set_target_properties(POSIX::clock_gettime PROPERTIES + INTERFACE_COMPILE_DEFINITIONS "${CMAKE_REQUIRED_DEFINITIONS}" + INTERFACE_LINK_LIBRARIES "${CMAKE_REQUIRED_LIBRARIES}" + ) + endif() +else() + if(${CMAKE_FIND_PACKAGE_NAME}_FIND_REQUIRED) + message(FATAL_ERROR "clock_gettime() not available.") + endif() +endif() + +cmake_pop_check_state() From a1ea09a1dd039bdc7e9e4b4e08d9831d9de5f7ad Mon Sep 17 00:00:00 2001 From: Raimo33 Date: Sat, 6 Sep 2025 23:45:57 +0200 Subject: [PATCH 2/4] bench: replace wall-clock timers with cpu-timers where possible This commit improves the reliability of benchmarks by removing some of the influence of other background running processes. This is achieved by using CPU bound clocks that aren't influenced by interrupts, sleeps, blocked I/O, etc. --- src/CMakeLists.txt | 20 +++++++++++++++----- src/bench.h | 4 ++-- src/tests_common.h | 47 +++++++++++++++++++++++++++++----------------- src/unit_test.c | 8 ++++---- 4 files changed, 51 insertions(+), 28 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ecbbbbe8e9..fb078a0970 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -124,13 +124,23 @@ elseif(CMAKE_SYSTEM_NAME STREQUAL "Windows") endif() unset(${PROJECT_NAME}_soversion) +if(SECP256K1_BUILD_BENCHMARK OR SECP256K1_BUILD_TESTS) + if(WIN32) + add_library(clock_interface INTERFACE) # It is empty on Windows. + else() + find_package(ClockGettime REQUIRED) + add_library(clock_interface ALIAS POSIX::clock_gettime) + endif() +endif() + + if(SECP256K1_BUILD_BENCHMARK) add_executable(bench bench.c) - target_link_libraries(bench secp256k1) + target_link_libraries(bench secp256k1 clock_interface) add_executable(bench_internal bench_internal.c) - target_link_libraries(bench_internal secp256k1_precomputed secp256k1_asm) + target_link_libraries(bench_internal secp256k1_precomputed secp256k1_asm clock_interface) add_executable(bench_ecmult bench_ecmult.c) - target_link_libraries(bench_ecmult secp256k1_precomputed secp256k1_asm) + target_link_libraries(bench_ecmult secp256k1_precomputed secp256k1_asm clock_interface) endif() if(SECP256K1_BUILD_TESTS) @@ -145,13 +155,13 @@ if(SECP256K1_BUILD_TESTS) endif() add_executable(noverify_tests tests.c) - target_link_libraries(noverify_tests secp256k1_precomputed secp256k1_asm) + target_link_libraries(noverify_tests secp256k1_precomputed secp256k1_asm clock_interface) target_compile_definitions(noverify_tests PRIVATE ${TEST_DEFINITIONS}) add_test(NAME secp256k1_noverify_tests COMMAND noverify_tests) if(NOT CMAKE_BUILD_TYPE STREQUAL "Coverage") add_executable(tests tests.c) target_compile_definitions(tests PRIVATE VERIFY ${TEST_DEFINITIONS}) - target_link_libraries(tests secp256k1_precomputed secp256k1_asm) + target_link_libraries(tests secp256k1_precomputed secp256k1_asm clock_interface) add_test(NAME secp256k1_tests COMMAND tests) endif() unset(TEST_DEFINITIONS) diff --git a/src/bench.h b/src/bench.h index 4e8e961b67..fb33e0eefc 100644 --- a/src/bench.h +++ b/src/bench.h @@ -85,9 +85,9 @@ static void run_benchmark(char *name, void (*benchmark)(void*, int), void (*setu if (setup != NULL) { setup(data); } - begin = gettime_i64(); + begin = gettime_us(); benchmark(data, iter); - total = gettime_i64() - begin; + total = gettime_us() - begin; if (teardown != NULL) { teardown(data, iter); } diff --git a/src/tests_common.h b/src/tests_common.h index a341633bbc..ce857d31b7 100644 --- a/src/tests_common.h +++ b/src/tests_common.h @@ -17,25 +17,38 @@ #include -#if (defined(_MSC_VER) && _MSC_VER >= 1900) -# include -#else -# include +#if defined(_WIN32) +# include +#else /* POSIX */ +# include #endif -static int64_t gettime_i64(void) { -#if (defined(_MSC_VER) && _MSC_VER >= 1900) - /* C11 way to get wallclock time */ - struct timespec tv; - if (!timespec_get(&tv, TIME_UTC)) { - fputs("timespec_get failed!", stderr); - exit(EXIT_FAILURE); - } - return (int64_t)tv.tv_nsec / 1000 + (int64_t)tv.tv_sec * 1000000LL; -#else - struct timeval tv; - gettimeofday(&tv, NULL); - return (int64_t)tv.tv_usec + (int64_t)tv.tv_sec * 1000000LL; +static int64_t gettime_us(void) { +#if defined(_WIN32) + + LARGE_INTEGER freq, counter; + QueryPerformanceFrequency(&freq); + QueryPerformanceCounter(&counter); + return (int64_t)(counter.QuadPart * 1000000 / freq.QuadPart); + +#else /* POSIX */ + +# if defined(CLOCK_PROCESS_CPUTIME_ID) + /* In theory, CLOCK_PROCESS_CPUTIME_ID is only useful if the process is locked to a core, + * see `man clock_gettime` on Linux. In practice, modern CPUs have synchronized TSCs which + * address this issue, see https://docs.amd.com/r/en-US/ug1586-onload-user/Timer-TSC-Stability . */ + const clockid_t clock_type = CLOCK_PROCESS_CPUTIME_ID; +# elif defined(CLOCK_MONOTONIC) + /* fallback to global timer */ + const clockid_t clock_type = CLOCK_MONOTONIC; +# else + /* fallback to wall-clock timer */ + const clockid_t clock_type = CLOCK_REALTIME; +# endif + + struct timespec ts; + clock_gettime(clock_type, &ts); + return (int64_t)ts.tv_sec * 1000000 + ts.tv_nsec / 1000; #endif } diff --git a/src/unit_test.c b/src/unit_test.c index a1858a117a..bd6ebc1a82 100644 --- a/src/unit_test.c +++ b/src/unit_test.c @@ -260,10 +260,10 @@ static int read_args(int argc, char** argv, int start, struct tf_framework* tf) } static void run_test_log(const struct tf_test_entry* t) { - int64_t start_time = gettime_i64(); + int64_t start_time = gettime_us(); printf("Running %s..\n", t->name); t->func(); - printf("Test %s PASSED (%.3f sec)\n", t->name, (double)(gettime_i64() - start_time) / 1000000); + printf("Test %s PASSED (%.3f sec)\n", t->name, (double)(gettime_us() - start_time) / 1000000); } static void run_test(const struct tf_test_entry* t) { t->func(); } @@ -416,7 +416,7 @@ static int tf_run(struct tf_framework* tf) { /* Loop iterator */ int it; /* Initial test time */ - int64_t start_time = gettime_i64(); + int64_t start_time = gettime_us(); /* Verify 'tf_init' has been called */ if (!tf->fn_run_test) { fprintf(stderr, "Error: No test runner set. You must call 'tf_init' first to initialize the framework " @@ -472,7 +472,7 @@ static int tf_run(struct tf_framework* tf) { } /* Print accumulated time */ - printf("Total execution time: %.3f seconds\n", (double)(gettime_i64() - start_time) / 1000000); + printf("Total execution time: %.3f seconds\n", (double)(gettime_us() - start_time) / 1000000); if (tf->fn_teardown && tf->fn_teardown() != 0) return EXIT_FAILURE; return status; From 76a4fc65e6cced083747258d876be1ef23e683b4 Mon Sep 17 00:00:00 2001 From: Raimo33 Date: Wed, 10 Sep 2025 11:56:03 +0200 Subject: [PATCH 3/4] bench: print clock info --- src/bench.c | 1 + src/bench.h | 8 ++++++++ src/bench_ecmult.c | 1 + src/bench_internal.c | 1 + 4 files changed, 11 insertions(+) diff --git a/src/bench.c b/src/bench.c index 8ba7623a0e..07c40a2358 100644 --- a/src/bench.c +++ b/src/bench.c @@ -252,6 +252,7 @@ int main(int argc, char** argv) { data.pubkeylen = 33; CHECK(secp256k1_ec_pubkey_serialize(data.ctx, data.pubkey, &data.pubkeylen, &pubkey, SECP256K1_EC_COMPRESSED) == 1); + print_clock_info(); print_output_table_header_row(); if (d || have_flag(argc, argv, "ecdsa") || have_flag(argc, argv, "verify") || have_flag(argc, argv, "ecdsa_verify")) run_benchmark("ecdsa_verify", bench_verify, NULL, NULL, &data, 10, iters); diff --git a/src/bench.h b/src/bench.h index fb33e0eefc..815a8396f2 100644 --- a/src/bench.h +++ b/src/bench.h @@ -156,6 +156,14 @@ static int get_iters(int default_iters) { } } +static void print_clock_info(void) { +#if defined(CLOCK_PROCESS_CPUTIME_ID) + printf("INFO: Using per-process CPU timer\n\n"); +#else + printf("WARN: Using global timer instead of per-process CPU timer.\n\n"); +#endif +} + static void print_output_table_header_row(void) { char* bench_str = "Benchmark"; /* left justified */ char* min_str = " Min(us) "; /* center alignment */ diff --git a/src/bench_ecmult.c b/src/bench_ecmult.c index b2bab65d26..c5375bd9c5 100644 --- a/src/bench_ecmult.c +++ b/src/bench_ecmult.c @@ -363,6 +363,7 @@ int main(int argc, char **argv) { secp256k1_ge_set_all_gej_var(data.pubkeys, data.pubkeys_gej, POINTS); + print_clock_info(); print_output_table_header_row(); /* Initialize offset1 and offset2 */ hash_into_offset(&data, 0); diff --git a/src/bench_internal.c b/src/bench_internal.c index 8688a4dc77..6b2e4bbf4b 100644 --- a/src/bench_internal.c +++ b/src/bench_internal.c @@ -398,6 +398,7 @@ int main(int argc, char **argv) { } } + print_clock_info(); print_output_table_header_row(); if (d || have_flag(argc, argv, "scalar") || have_flag(argc, argv, "half")) run_benchmark("scalar_half", bench_scalar_half, bench_setup, NULL, &data, 10, iters*100); From e14981e28e1c1e4b2fb6321cd94342d0b2849be7 Mon Sep 17 00:00:00 2001 From: Raimo33 Date: Wed, 10 Sep 2025 11:57:29 +0200 Subject: [PATCH 4/4] docs: add benchmarking best practices --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 90edae1a2c..869c49322f 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,8 @@ Benchmark ------------ If configured with `--enable-benchmark` (which is the default), binaries for benchmarking the libsecp256k1 functions will be present in the root directory after the build. +For reliable benchmarks, it is strongly recommended to pin the process to a single CPU core and to disable CPU frequency scaling. + To print the benchmark result to the command line: $ ./bench_name