diff mbox series

[2/4] tests: benchmark crypto with fixed data size, not time period

Message ID 20191017145654.11371-3-berrange@redhat.com
State New
Headers show
Series crypto: improve performance of ciphers in XTS mode | expand

Commit Message

Daniel P. Berrangé Oct. 17, 2019, 2:56 p.m. UTC 
Currently the crypto benchmarks are processing data in varying chunk
sizes, over a fixed time period. This turns out to be a terrible idea
because with small chunk sizes the overhead of checking the elapsed
time on each loop iteration masks the true performance.

Benchmarking over a fixed data size avoids the loop running any system
calls which can interfere with the performance measurements.

Before this change

Enc chunk 512 bytes 2283.47 MB/sec Dec chunk 512 bytes 2236.23 MB/sec OK
Enc chunk 4096 bytes 2744.97 MB/sec Dec chunk 4096 bytes 2614.71 MB/sec OK
Enc chunk 16384 bytes 2777.53 MB/sec Dec chunk 16384 bytes 2678.44 MB/sec OK
Enc chunk 65536 bytes 2809.34 MB/sec Dec chunk 65536 bytes 2699.47 MB/sec OK

After this change

Enc chunk 512 bytes 2058.22 MB/sec Dec chunk 512 bytes 2030.11 MB/sec OK
Enc chunk 4096 bytes 2699.27 MB/sec Dec chunk 4096 bytes 2573.78 MB/sec OK
Enc chunk 16384 bytes 2748.52 MB/sec Dec chunk 16384 bytes 2653.76 MB/sec OK
Enc chunk 65536 bytes 2814.08 MB/sec Dec chunk 65536 bytes 2712.74 MB/sec OK

The actual crypto performance hasn't changed, which shows how
significant the mis-measurement has been for small data sizes.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/benchmark-crypto-cipher.c | 26 ++++++++++++++------------
 tests/benchmark-crypto-hash.c   | 17 +++++++++--------
 2 files changed, 23 insertions(+), 20 deletions(-)

Comments

Philippe Mathieu-Daudé Oct. 25, 2019, 1:36 p.m. UTC | #1 
On 10/17/19 4:56 PM, Daniel P. Berrangé wrote:
> Currently the crypto benchmarks are processing data in varying chunk
> sizes, over a fixed time period. This turns out to be a terrible idea
> because with small chunk sizes the overhead of checking the elapsed
> time on each loop iteration masks the true performance.
> 
> Benchmarking over a fixed data size avoids the loop running any system
> calls which can interfere with the performance measurements.
> 
> Before this change
> 
> Enc chunk 512 bytes 2283.47 MB/sec Dec chunk 512 bytes 2236.23 MB/sec OK
> Enc chunk 4096 bytes 2744.97 MB/sec Dec chunk 4096 bytes 2614.71 MB/sec OK
> Enc chunk 16384 bytes 2777.53 MB/sec Dec chunk 16384 bytes 2678.44 MB/sec OK
> Enc chunk 65536 bytes 2809.34 MB/sec Dec chunk 65536 bytes 2699.47 MB/sec OK
> 
> After this change
> 
> Enc chunk 512 bytes 2058.22 MB/sec Dec chunk 512 bytes 2030.11 MB/sec OK
> Enc chunk 4096 bytes 2699.27 MB/sec Dec chunk 4096 bytes 2573.78 MB/sec OK
> Enc chunk 16384 bytes 2748.52 MB/sec Dec chunk 16384 bytes 2653.76 MB/sec OK
> Enc chunk 65536 bytes 2814.08 MB/sec Dec chunk 65536 bytes 2712.74 MB/sec OK
> 
> The actual crypto performance hasn't changed, which shows how
> significant the mis-measurement has been for small data sizes.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/benchmark-crypto-cipher.c | 26 ++++++++++++++------------
>   tests/benchmark-crypto-hash.c   | 17 +++++++++--------
>   2 files changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/benchmark-crypto-cipher.c b/tests/benchmark-crypto-cipher.c
> index 3ca31a2779..d8db5504d4 100644
> --- a/tests/benchmark-crypto-cipher.c
> +++ b/tests/benchmark-crypto-cipher.c
> @@ -21,11 +21,12 @@ static void test_cipher_speed(size_t chunk_size,
>   {
>       QCryptoCipher *cipher;
>       Error *err = NULL;
> -    double total = 0.0;
>       uint8_t *key = NULL, *iv = NULL;
>       uint8_t *plaintext = NULL, *ciphertext = NULL;
>       size_t nkey;
>       size_t niv;
> +    size_t total = 2 * GiB;

Can you use 'const size_t total'?.

> +    size_t remain;
>   
>       if (!qcrypto_cipher_supports(alg, mode)) {
>           return;
> @@ -58,33 +59,34 @@ static void test_cipher_speed(size_t chunk_size,
>                                         &err) == 0);
>   
>       g_test_timer_start();
> -    do {
> +    remain = total;
> +    while (remain) {
>           g_assert(qcrypto_cipher_encrypt(cipher,
>                                           plaintext,
>                                           ciphertext,
>                                           chunk_size,
>                                           &err) == 0);
> -        total += chunk_size;
> -    } while (g_test_timer_elapsed() < 1.0);
> +        remain -= chunk_size;
> +    }
> +    g_test_timer_elapsed();
>   
> -    total /= MiB;
>       g_print("Enc chunk %zu bytes ", chunk_size);
> -    g_print("%.2f MB/sec ", total / g_test_timer_last());
> +    g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last());
>   
> -    total = 0.0;
>       g_test_timer_start();
> -    do {
> +    remain = total;
> +    while (remain) {
>           g_assert(qcrypto_cipher_decrypt(cipher,
>                                           plaintext,
>                                           ciphertext,
>                                           chunk_size,
>                                           &err) == 0);
> -        total += chunk_size;
> -    } while (g_test_timer_elapsed() < 1.0);
> +        remain -= chunk_size;
> +    }
> +    g_test_timer_elapsed();
>   
> -    total /= MiB;
>       g_print("Dec chunk %zu bytes ", chunk_size);
> -    g_print("%.2f MB/sec ", total / g_test_timer_last());
> +    g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last());
>   
>       qcrypto_cipher_free(cipher);
>       g_free(plaintext);
> diff --git a/tests/benchmark-crypto-hash.c b/tests/benchmark-crypto-hash.c
> index 9b6f7a9155..67b7067223 100644
> --- a/tests/benchmark-crypto-hash.c
> +++ b/tests/benchmark-crypto-hash.c
> @@ -20,7 +20,8 @@ static void test_hash_speed(const void *opaque)
>       size_t chunk_size = (size_t)opaque;
>       uint8_t *in = NULL, *out = NULL;
>       size_t out_len = 0;
> -    double total = 0.0;
> +    size_t total = 2 * GiB;

Ditto: 'const size_t total'.

> +    size_t remain;
>       struct iovec iov;
>       int ret;
>   
> @@ -31,20 +32,20 @@ static void test_hash_speed(const void *opaque)
>       iov.iov_len = chunk_size;
>   
>       g_test_timer_start();
> -    do {
> +    remain = total;
> +    while (remain) {
>           ret = qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256,
>                                     &iov, 1, &out, &out_len,
>                                     NULL);
>           g_assert(ret == 0);
>   
> -        total += chunk_size;
> -    } while (g_test_timer_elapsed() < 5.0);
> +        remain -= chunk_size;
> +    }
> +    g_test_timer_elapsed();
>   
> -    total /= MiB;
>       g_print("sha256: ");
> -    g_print("Testing chunk_size %zu bytes ", chunk_size);
> -    g_print("done: %.2f MB in %.2f secs: ", total, g_test_timer_last());
> -    g_print("%.2f MB/sec\n", total / g_test_timer_last());
> +    g_print("Hash %zu GB chunk size %zu bytes ", total / GiB, chunk_size);
> +    g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last());
>   
>       g_free(out);
>       g_free(in);
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Stefano Garzarella Oct. 25, 2019, 1:46 p.m. UTC | #2 
On Thu, Oct 17, 2019 at 03:56:52PM +0100, Daniel P. Berrangé wrote:
> Currently the crypto benchmarks are processing data in varying chunk
> sizes, over a fixed time period. This turns out to be a terrible idea
> because with small chunk sizes the overhead of checking the elapsed
> time on each loop iteration masks the true performance.
> 
> Benchmarking over a fixed data size avoids the loop running any system
> calls which can interfere with the performance measurements.
> 
> Before this change
> 
> Enc chunk 512 bytes 2283.47 MB/sec Dec chunk 512 bytes 2236.23 MB/sec OK
> Enc chunk 4096 bytes 2744.97 MB/sec Dec chunk 4096 bytes 2614.71 MB/sec OK
> Enc chunk 16384 bytes 2777.53 MB/sec Dec chunk 16384 bytes 2678.44 MB/sec OK
> Enc chunk 65536 bytes 2809.34 MB/sec Dec chunk 65536 bytes 2699.47 MB/sec OK
> 
> After this change
> 
> Enc chunk 512 bytes 2058.22 MB/sec Dec chunk 512 bytes 2030.11 MB/sec OK
> Enc chunk 4096 bytes 2699.27 MB/sec Dec chunk 4096 bytes 2573.78 MB/sec OK
> Enc chunk 16384 bytes 2748.52 MB/sec Dec chunk 16384 bytes 2653.76 MB/sec OK
> Enc chunk 65536 bytes 2814.08 MB/sec Dec chunk 65536 bytes 2712.74 MB/sec OK
> 
> The actual crypto performance hasn't changed, which shows how
> significant the mis-measurement has been for small data sizes.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/benchmark-crypto-cipher.c | 26 ++++++++++++++------------
>  tests/benchmark-crypto-hash.c   | 17 +++++++++--------
>  2 files changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/benchmark-crypto-cipher.c b/tests/benchmark-crypto-cipher.c
> index 3ca31a2779..d8db5504d4 100644
> --- a/tests/benchmark-crypto-cipher.c
> +++ b/tests/benchmark-crypto-cipher.c
> @@ -21,11 +21,12 @@ static void test_cipher_speed(size_t chunk_size,
>  {
>      QCryptoCipher *cipher;
>      Error *err = NULL;
> -    double total = 0.0;
>      uint8_t *key = NULL, *iv = NULL;
>      uint8_t *plaintext = NULL, *ciphertext = NULL;
>      size_t nkey;
>      size_t niv;
> +    size_t total = 2 * GiB;
> +    size_t remain;
>  
>      if (!qcrypto_cipher_supports(alg, mode)) {
>          return;
> @@ -58,33 +59,34 @@ static void test_cipher_speed(size_t chunk_size,
>                                        &err) == 0);
>  
>      g_test_timer_start();
> -    do {
> +    remain = total;
> +    while (remain) {
>          g_assert(qcrypto_cipher_encrypt(cipher,
>                                          plaintext,
>                                          ciphertext,
>                                          chunk_size,
>                                          &err) == 0);
> -        total += chunk_size;
> -    } while (g_test_timer_elapsed() < 1.0);
> +        remain -= chunk_size;
> +    }
> +    g_test_timer_elapsed();
>  
> -    total /= MiB;
>      g_print("Enc chunk %zu bytes ", chunk_size);
> -    g_print("%.2f MB/sec ", total / g_test_timer_last());
> +    g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last());
>  
> -    total = 0.0;
>      g_test_timer_start();
> -    do {
> +    remain = total;
> +    while (remain) {
>          g_assert(qcrypto_cipher_decrypt(cipher,
>                                          plaintext,
>                                          ciphertext,
>                                          chunk_size,
>                                          &err) == 0);
> -        total += chunk_size;
> -    } while (g_test_timer_elapsed() < 1.0);
> +        remain -= chunk_size;

Are we sure that total is a multiple of chunk_size?
Maybe I would have increased 'done' and checked (done <= total),
but maybe it doesn't matter since I think that chunk_size will always
be a power of two.

> +    }
> +    g_test_timer_elapsed();
>  
> -    total /= MiB;
>      g_print("Dec chunk %zu bytes ", chunk_size);
> -    g_print("%.2f MB/sec ", total / g_test_timer_last());
> +    g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last());
>  
>      qcrypto_cipher_free(cipher);
>      g_free(plaintext);
> diff --git a/tests/benchmark-crypto-hash.c b/tests/benchmark-crypto-hash.c
> index 9b6f7a9155..67b7067223 100644
> --- a/tests/benchmark-crypto-hash.c
> +++ b/tests/benchmark-crypto-hash.c
> @@ -20,7 +20,8 @@ static void test_hash_speed(const void *opaque)
>      size_t chunk_size = (size_t)opaque;
>      uint8_t *in = NULL, *out = NULL;
>      size_t out_len = 0;
> -    double total = 0.0;
> +    size_t total = 2 * GiB;
> +    size_t remain;
>      struct iovec iov;
>      int ret;
>  
> @@ -31,20 +32,20 @@ static void test_hash_speed(const void *opaque)
>      iov.iov_len = chunk_size;
>  
>      g_test_timer_start();
> -    do {
> +    remain = total;
> +    while (remain) {
>          ret = qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256,
>                                    &iov, 1, &out, &out_len,
>                                    NULL);
>          g_assert(ret == 0);
>  
> -        total += chunk_size;
> -    } while (g_test_timer_elapsed() < 5.0);
> +        remain -= chunk_size;
> +    }
> +    g_test_timer_elapsed();
>  
> -    total /= MiB;
>      g_print("sha256: ");
> -    g_print("Testing chunk_size %zu bytes ", chunk_size);
> -    g_print("done: %.2f MB in %.2f secs: ", total, g_test_timer_last());
> -    g_print("%.2f MB/sec\n", total / g_test_timer_last());
> +    g_print("Hash %zu GB chunk size %zu bytes ", total / GiB, chunk_size);
> +    g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last());
>  
>      g_free(out);
>      g_free(in);

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
diff mbox series

Patch

diff --git a/tests/benchmark-crypto-cipher.c b/tests/benchmark-crypto-cipher.c
index 3ca31a2779..d8db5504d4 100644
--- a/tests/benchmark-crypto-cipher.c
+++ b/tests/benchmark-crypto-cipher.c
@@ -21,11 +21,12 @@  static void test_cipher_speed(size_t chunk_size,
 {
     QCryptoCipher *cipher;
     Error *err = NULL;
-    double total = 0.0;
     uint8_t *key = NULL, *iv = NULL;
     uint8_t *plaintext = NULL, *ciphertext = NULL;
     size_t nkey;
     size_t niv;
+    size_t total = 2 * GiB;
+    size_t remain;
 
     if (!qcrypto_cipher_supports(alg, mode)) {
         return;
@@ -58,33 +59,34 @@  static void test_cipher_speed(size_t chunk_size,
                                       &err) == 0);
 
     g_test_timer_start();
-    do {
+    remain = total;
+    while (remain) {
         g_assert(qcrypto_cipher_encrypt(cipher,
                                         plaintext,
                                         ciphertext,
                                         chunk_size,
                                         &err) == 0);
-        total += chunk_size;
-    } while (g_test_timer_elapsed() < 1.0);
+        remain -= chunk_size;
+    }
+    g_test_timer_elapsed();
 
-    total /= MiB;
     g_print("Enc chunk %zu bytes ", chunk_size);
-    g_print("%.2f MB/sec ", total / g_test_timer_last());
+    g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last());
 
-    total = 0.0;
     g_test_timer_start();
-    do {
+    remain = total;
+    while (remain) {
         g_assert(qcrypto_cipher_decrypt(cipher,
                                         plaintext,
                                         ciphertext,
                                         chunk_size,
                                         &err) == 0);
-        total += chunk_size;
-    } while (g_test_timer_elapsed() < 1.0);
+        remain -= chunk_size;
+    }
+    g_test_timer_elapsed();
 
-    total /= MiB;
     g_print("Dec chunk %zu bytes ", chunk_size);
-    g_print("%.2f MB/sec ", total / g_test_timer_last());
+    g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last());
 
     qcrypto_cipher_free(cipher);
     g_free(plaintext);
diff --git a/tests/benchmark-crypto-hash.c b/tests/benchmark-crypto-hash.c
index 9b6f7a9155..67b7067223 100644
--- a/tests/benchmark-crypto-hash.c
+++ b/tests/benchmark-crypto-hash.c
@@ -20,7 +20,8 @@  static void test_hash_speed(const void *opaque)
     size_t chunk_size = (size_t)opaque;
     uint8_t *in = NULL, *out = NULL;
     size_t out_len = 0;
-    double total = 0.0;
+    size_t total = 2 * GiB;
+    size_t remain;
     struct iovec iov;
     int ret;
 
@@ -31,20 +32,20 @@  static void test_hash_speed(const void *opaque)
     iov.iov_len = chunk_size;
 
     g_test_timer_start();
-    do {
+    remain = total;
+    while (remain) {
         ret = qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256,
                                   &iov, 1, &out, &out_len,
                                   NULL);
         g_assert(ret == 0);
 
-        total += chunk_size;
-    } while (g_test_timer_elapsed() < 5.0);
+        remain -= chunk_size;
+    }
+    g_test_timer_elapsed();
 
-    total /= MiB;
     g_print("sha256: ");
-    g_print("Testing chunk_size %zu bytes ", chunk_size);
-    g_print("done: %.2f MB in %.2f secs: ", total, g_test_timer_last());
-    g_print("%.2f MB/sec\n", total / g_test_timer_last());
+    g_print("Hash %zu GB chunk size %zu bytes ", total / GiB, chunk_size);
+    g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last());
 
     g_free(out);
     g_free(in);