Merge dashpay#722: Context isn't freed in the ECDH benchmark

85b35af Add running benchmarks regularly and under valgrind in travis (Elichai Turkel)
ca4906b Pass num of iters to benchmarks as variable, and define envvar (Elichai Turkel)
02dd5f1 free the ctx at the end of bench_ecdh (Elichai Turkel)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK 85b35af I looked at the diff and tested the ecdh benchmark using valgrind
  jonasnick:
    ACK 85b35af

Tree-SHA512: 029456d2c8f6c2c45c689279683ae30b067872bbfaee76a657f7fc3a059e2dffcde09ed29e3610de3adb055405126b6c3656c7ab5f5aaa1f45af2b32d4693ec4

.travis.yml

@@ -11,7 +11,7 @@ compiler:
   - gcc
 env:
   global:
-    - FIELD=auto  BIGNUM=auto  SCALAR=auto  ENDOMORPHISM=no  STATICPRECOMPUTATION=yes  ECMULTGENPRECISION=auto  ASM=no  BUILD=check  EXTRAFLAGS=  HOST=  ECDH=no  RECOVERY=no  EXPERIMENTAL=no CTIMETEST=yes
+    - FIELD=auto  BIGNUM=auto  SCALAR=auto  ENDOMORPHISM=no  STATICPRECOMPUTATION=yes  ECMULTGENPRECISION=auto  ASM=no  BUILD=check  EXTRAFLAGS=  HOST=  ECDH=no  RECOVERY=no  EXPERIMENTAL=no CTIMETEST=yes BENCH=yes SECP256K1_BENCH_ITERS=2
   matrix:
     - SCALAR=32bit    RECOVERY=yes
     - SCALAR=32bit    FIELD=32bit       ECDH=yes  EXPERIMENTAL=yes
@@ -25,7 +25,7 @@ env:
     - BIGNUM=no
     - BIGNUM=no       ENDOMORPHISM=yes RECOVERY=yes EXPERIMENTAL=yes
     - BIGNUM=no       STATICPRECOMPUTATION=no
-    - BUILD=distcheck CTIMETEST=
+    - BUILD=distcheck CTIMETEST= BENCH=
     - EXTRAFLAGS=CPPFLAGS=-DDETERMINISTIC
     - EXTRAFLAGS=CFLAGS=-O0
     - ECMULTGENPRECISION=2
@@ -94,6 +94,12 @@ script:
    travis_wait 30 valgrind --error-exitcode=42 ./tests 16 &&
    travis_wait 30 valgrind --error-exitcode=42 ./exhaustive_tests;
    fi
+ - if [ -n "$BENCH" ]; then
+     if [ -n "$VALGRIND" ]; then EXEC='libtool --mode=execute valgrind --error-exitcode=42'; else EXEC= ; fi &&
+     $EXEC ./bench_ecmult &>> bench.log && $EXEC ./bench_internal &>> bench.log && $EXEC ./bench_sign &>> bench.log && $EXEC ./bench_verify &>> bench.log &&
+     if [ "$RECOVERY" == "yes" ]; then $EXEC ./bench_recover &>> bench.log; fi &&
+     if [ "$ECDH" == "yes" ]; then $EXEC ./bench_ecdh &>> bench.log; fi;
+   fi
  - if [ -n "$CTIMETEST" ]; then
    libtool --mode=execute valgrind  ./valgrind_ctime_test &> valgrind_ctime_test.log;
    fi
@@ -102,3 +108,4 @@ after_script:
     - cat ./tests.log
     - cat ./exhaustive_tests.log
     - cat ./valgrind_ctime_test.log
+    - cat ./bench.log

src/bench.h

@@ -73,7 +73,7 @@ void print_number(const int64_t x) {
     printf("%s", &buffer[ptr]);
 }
 
-void run_benchmark(char *name, void (*benchmark)(void*), void (*setup)(void*), void (*teardown)(void*), void* data, int count, int iter) {
+void run_benchmark(char *name, void (*benchmark)(void*, int), void (*setup)(void*), void (*teardown)(void*, int), void* data, int count, int iter) {
     int i;
     int64_t min = INT64_MAX;
     int64_t sum = 0;
@@ -84,10 +84,10 @@ void run_benchmark(char *name, void (*benchmark)(void*), void (*setup)(void*), v
             setup(data);
         }
         begin = gettime_i64();
-        benchmark(data);
+        benchmark(data, iter);
         total = gettime_i64() - begin;
         if (teardown != NULL) {
-            teardown(data);
+            teardown(data, iter);
         }
         if (total < min) {
             min = total;
@@ -121,4 +121,13 @@ int have_flag(int argc, char** argv, char *flag) {
     return 0;
 }
 
+int get_iters(int default_iters) {
+    char* env = getenv("SECP256K1_BENCH_ITERS");
+    if (env) {
+        return strtol(env, NULL, 0);
+    } else {
+        return default_iters;
+    }
+}
+
 #endif /* SECP256K1_BENCH_H */

src/bench_ecdh.c

@@ -28,27 +28,32 @@ static void bench_ecdh_setup(void* arg) {
         0xa2, 0xba, 0xd1, 0x84, 0xf8, 0x83, 0xc6, 0x9f
     };
 
-    /* create a context with no capabilities */
-    data->ctx = secp256k1_context_create(SECP256K1_FLAGS_TYPE_CONTEXT);
     for (i = 0; i < 32; i++) {
         data->scalar[i] = i + 1;
     }
     CHECK(secp256k1_ec_pubkey_parse(data->ctx, &data->point, point, sizeof(point)) == 1);
 }
 
-static void bench_ecdh(void* arg) {
+static void bench_ecdh(void* arg, int iters) {
     int i;
     unsigned char res[32];
     bench_ecdh_data *data = (bench_ecdh_data*)arg;
 
-    for (i = 0; i < 20000; i++) {
+    for (i = 0; i < iters; i++) {
         CHECK(secp256k1_ecdh(data->ctx, res, &data->point, data->scalar, NULL, NULL) == 1);
     }
 }
 
 int main(void) {
     bench_ecdh_data data;
 
-    run_benchmark("ecdh", bench_ecdh, bench_ecdh_setup, NULL, &data, 10, 20000);
+    int iters = get_iters(20000);
+
+    /* create a context with no capabilities */
+    data.ctx = secp256k1_context_create(SECP256K1_FLAGS_TYPE_CONTEXT);
+
+    run_benchmark("ecdh", bench_ecdh, bench_ecdh_setup, NULL, &data, 10, iters);
+
+    secp256k1_context_destroy(data.ctx);
     return 0;
 }

src/bench_ecmult.c

@@ -18,7 +18,6 @@
 #include "secp256k1.c"
 
 #define POINTS 32768
-#define ITERS 10000
 
 typedef struct {
     /* Setup once in advance */
@@ -55,13 +54,13 @@ static int bench_callback(secp256k1_scalar* sc, secp256k1_ge* ge, size_t idx, vo
     return 1;
 }
 
-static void bench_ecmult(void* arg) {
+static void bench_ecmult(void* arg, int iters) {
     bench_data* data = (bench_data*)arg;
 
-    size_t count = data->count;
     int includes_g = data->includes_g;
-    size_t iters = 1 + ITERS / count;
-    size_t iter;
+    int iter;
+    int count = data->count;
+    iters = iters / data->count;
 
     for (iter = 0; iter < iters; ++iter) {
         data->ecmult_multi(&data->ctx->error_callback, &data->ctx->ecmult_ctx, data->scratch, &data->output[iter], data->includes_g ? &data->scalars[data->offset1] : NULL, bench_callback, arg, count - includes_g);
@@ -76,10 +75,10 @@ static void bench_ecmult_setup(void* arg) {
     data->offset2 = (data->count * 0x7f6f537b + 0x6a1a8f49) % POINTS;
 }
 
-static void bench_ecmult_teardown(void* arg) {
+static void bench_ecmult_teardown(void* arg, int iters) {
     bench_data* data = (bench_data*)arg;
-    size_t iters = 1 + ITERS / data->count;
-    size_t iter;
+    int iter;
+    iters = iters / data->count;
     /* Verify the results in teardown, to avoid doing comparisons while benchmarking. */
     for (iter = 0; iter < iters; ++iter) {
         secp256k1_gej tmp;
@@ -104,10 +103,10 @@ static void generate_scalar(uint32_t num, secp256k1_scalar* scalar) {
     CHECK(!overflow);
 }
 
-static void run_test(bench_data* data, size_t count, int includes_g) {
+static void run_test(bench_data* data, size_t count, int includes_g, int num_iters) {
     char str[32];
     static const secp256k1_scalar zero = SECP256K1_SCALAR_CONST(0, 0, 0, 0, 0, 0, 0, 0);
-    size_t iters = 1 + ITERS / count;
+    size_t iters = 1 + num_iters / count;
     size_t iter;
 
     data->count = count;
@@ -130,7 +129,7 @@ static void run_test(bench_data* data, size_t count, int includes_g) {
 
     /* Run the benchmark. */
     sprintf(str, includes_g ? "ecmult_%ig" : "ecmult_%i", (int)count);
-    run_benchmark(str, bench_ecmult, bench_ecmult_setup, bench_ecmult_teardown, data, 10, count * (1 + ITERS / count));
+    run_benchmark(str, bench_ecmult, bench_ecmult_setup, bench_ecmult_teardown, data, 10, count * iters);
 }
 
 int main(int argc, char **argv) {
@@ -139,6 +138,8 @@ int main(int argc, char **argv) {
     secp256k1_gej* pubkeys_gej;
     size_t scratch_size;
 
+    int iters = get_iters(10000);
+
     data.ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
     scratch_size = secp256k1_strauss_scratch_size(POINTS) + STRAUSS_SCRATCH_OBJECTS*16;
     data.scratch = secp256k1_scratch_space_create(data.ctx, scratch_size);
@@ -167,8 +168,8 @@ int main(int argc, char **argv) {
     data.scalars = malloc(sizeof(secp256k1_scalar) * POINTS);
     data.seckeys = malloc(sizeof(secp256k1_scalar) * POINTS);
     data.pubkeys = malloc(sizeof(secp256k1_ge) * POINTS);
-    data.expected_output = malloc(sizeof(secp256k1_gej) * (ITERS + 1));
-    data.output = malloc(sizeof(secp256k1_gej) * (ITERS + 1));
+    data.expected_output = malloc(sizeof(secp256k1_gej) * (iters + 1));
+    data.output = malloc(sizeof(secp256k1_gej) * (iters + 1));
 
     /* Generate a set of scalars, and private/public keypairs. */
     pubkeys_gej = malloc(sizeof(secp256k1_gej) * POINTS);
@@ -185,14 +186,20 @@ int main(int argc, char **argv) {
     free(pubkeys_gej);
 
     for (i = 1; i <= 8; ++i) {
-        run_test(&data, i, 1);
+        run_test(&data, i, 1, iters);
     }
 
-    for (p = 0; p <= 11; ++p) {
-        for (i = 9; i <= 16; ++i) {
-            run_test(&data, i << p, 1);
+    /* This is disabled with low count of iterations because the loop runs 77 times even with iters=1
+    * and the higher it goes the longer the computation takes(more points)
+    * So we don't run this benchmark with low iterations to prevent slow down */
+     if (iters > 2) {
+        for (p = 0; p <= 11; ++p) {
+            for (i = 9; i <= 16; ++i) {
+                run_test(&data, i << p, 1, iters);
+            }
         }
     }
+
     if (data.scratch != NULL) {
         secp256k1_scratch_space_destroy(data.ctx, data.scratch);
     }