diff mbox

Add malloc micro benchmark

Message ID AM6PR08MB5078D413D83D884D2FAFFDA683470@AM6PR08MB5078.eurprd08.prod.outlook.com
State Committed, archived
Headers show

Commit Message

Wilco Dijkstra March 18, 2019, 5:16 p.m. UTC
ping
  

Hi Carlos,

> BUILDSTDERR: bench-malloc-simple.c: In function 'bench':
> BUILDSTDERR: bench-malloc-simple.c:89:17: error: variable 'res' set but not used [-Werror=unused-but-set-variable]
> BUILDSTDERR:    89 |   unsigned long res;
> BUILDSTDERR:       |                 ^~~
> BUILDSTDERR: cc1: all warnings being treated as errors
>
> Affects aarch64, armv7hl, and s390x.
> 
> I assume we need a "(void) res" like we have in bench-malloc-thread.c?
> 
> I'm going to checkin a quick fix to Rawhide and report back if anything
> else breaks.

Does that enable extra errors somehow? I can't reproduce it.

Anyway TIMING_INIT is redundant for bench-malloc-*.c, so here's a
patch to just kill it:


Remove TIMING_INIT since it's only used in bench-skeleton.c if there
is no hp-timing support (which will become the default after [1]).

[1] https://sourceware.org/ml/libc-alpha/2019-02/msg00468.html

ChangeLog:
2019-03-04  Wilco Dijkstra  <wdijkstr@arm.com>

        * benchtests/bench-malloc-simple.c: Remove TIMING_INIT.
        * benchtests/bench-malloc-thread.c: Likewise.
        * benchtests/bench-skeleton.c: Likewise.
        * benchtests/bench-strtod.c: Likewise.
        * benchtests/bench-timing.h: Likewise.

--

Comments

Carlos O'Donell April 9, 2019, 5:25 a.m. UTC | #1
On 3/18/19 1:16 PM, Wilco Dijkstra wrote:
> ping
>    
> 
> Hi Carlos,
> 
>> BUILDSTDERR: bench-malloc-simple.c: In function 'bench':
>> BUILDSTDERR: bench-malloc-simple.c:89:17: error: variable 'res' set but not used [-Werror=unused-but-set-variable]
>> BUILDSTDERR:    89 |   unsigned long res;
>> BUILDSTDERR:       |                 ^~~
>> BUILDSTDERR: cc1: all warnings being treated as errors
>>
>> Affects aarch64, armv7hl, and s390x.
>>
>> I assume we need a "(void) res" like we have in bench-malloc-thread.c?
>>
>> I'm going to checkin a quick fix to Rawhide and report back if anything
>> else breaks.
> 
> Does that enable extra errors somehow? I can't reproduce it.
> 
> Anyway TIMING_INIT is redundant for bench-malloc-*.c, so here's a
> patch to just kill it:

LGTM.

Sorry for the delay.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> 
> Remove TIMING_INIT since it's only used in bench-skeleton.c if there
> is no hp-timing support (which will become the default after [1]).
> 
> [1] https://sourceware.org/ml/libc-alpha/2019-02/msg00468.html
> 
> ChangeLog:
> 2019-03-04  Wilco Dijkstra  <wdijkstr@arm.com>
> 
>          * benchtests/bench-malloc-simple.c: Remove TIMING_INIT.
>          * benchtests/bench-malloc-thread.c: Likewise.
>          * benchtests/bench-skeleton.c: Likewise.
>          * benchtests/bench-strtod.c: Likewise.
>          * benchtests/bench-timing.h: Likewise.
> 
> --
> 
> diff --git a/benchtests/bench-malloc-simple.c b/benchtests/bench-malloc-simple.c
> index 83203ff3187654a1710c9ef81016f854957b9d64..b8bb2cc116953c6691c17633d18c5661c7d9243e 100644
> --- a/benchtests/bench-malloc-simple.c
> +++ b/benchtests/bench-malloc-simple.c
> @@ -86,9 +86,6 @@ bench (unsigned long size)
>   {
>     size_t iters = NUM_ITERS;
>     int **arr = (int**) malloc (MAX_ALLOCS * sizeof (void*));
> -  unsigned long res;
> -
> -  TIMING_INIT (res);

OK.

>   
>     for (int t = 0; t <= 3; t++)
>       for (int i = 0; i < NUM_ALLOCS; i++)
> diff --git a/benchtests/bench-malloc-thread.c b/benchtests/bench-malloc-thread.c
> index bb4ba727a88059ecbe7305f5b8ad1693c1f1f266..52261425b0f1af32c17328ea5e0a5bb6f230df47 100644
> --- a/benchtests/bench-malloc-thread.c
> +++ b/benchtests/bench-malloc-thread.c
> @@ -225,7 +225,6 @@ main (int argc, char **argv)
>   {
>     timing_t cur;
>     size_t iters = 0, num_threads = 1;
> -  unsigned long res;

OK.

>     json_ctx_t json_ctx;
>     double d_total_s, d_total_i;
>     struct sigaction act;
> @@ -261,10 +260,6 @@ main (int argc, char **argv)
>   
>     json_attr_object_begin (&json_ctx, "");
>   
> -  TIMING_INIT (res);
> -
> -  (void) res;

OK.

> -
>     memset (&act, 0, sizeof (act));
>     act.sa_handler = &alarm_handler;
>   
> diff --git a/benchtests/bench-skeleton.c b/benchtests/bench-skeleton.c
> index 37625c4296882268f6260d99adbc7f0295164ffc..854151e5a82028e74fe3a966e82004572542f411 100644
> --- a/benchtests/bench-skeleton.c
> +++ b/benchtests/bench-skeleton.c
> @@ -48,14 +48,11 @@ main (int argc, char **argv)
>   
>     memset (&runtime, 0, sizeof (runtime));
>   
> -  unsigned long iters, res;
> +  unsigned long iters = 1000;

OK. A fixed number of iterations will do.

>   
>   #ifdef BENCH_INIT
>     BENCH_INIT ();
>   #endif
> -  TIMING_INIT (res);
> -
> -  iters = 1000 * res;

OK.

>   
>     json_init (&json_ctx, 2, stdout);
>   
> diff --git a/benchtests/bench-strtod.c b/benchtests/bench-strtod.c
> index 4de0b9acb67eb925a80249322957ce8b3c08c8d6..d5b2503553ef74f33cace919ae9c62f79cd11c9c 100644
> --- a/benchtests/bench-strtod.c
> +++ b/benchtests/bench-strtod.c
> @@ -89,9 +89,6 @@ int
>   do_bench (void)
>   {
>     const size_t iters = INNER_LOOP_ITERS;
> -  timing_t res __attribute__ ((unused));
> -
> -  TIMING_INIT (res);

OK.

>   
>     for (size_t i = 0; inputs[i] != NULL; ++i)
>       {
> diff --git a/benchtests/bench-timing.h b/benchtests/bench-timing.h
> index 41b7324527b9deed67b3479cb1308fbd291bc5ca..f9b19fcd29efb45ea02c375e37caba94c93956d1 100644
> --- a/benchtests/bench-timing.h
> +++ b/benchtests/bench-timing.h
> @@ -28,8 +28,6 @@ typedef hp_timing_t timing_t;
>   
>   # define TIMING_TYPE "hp_timing"
>   
> -# define TIMING_INIT(res) ({ (res) = 1; })

OK.

> -
>   # define TIMING_NOW(var) HP_TIMING_NOW (var)
>   # define TIMING_DIFF(diff, start, end) HP_TIMING_DIFF ((diff), (start), (end))
>   # define TIMING_ACCUM(sum, diff) HP_TIMING_ACCUM_NT ((sum), (diff))
> @@ -41,15 +39,6 @@ typedef uint64_t timing_t;
>   
>   # define TIMING_TYPE "clock_gettime"
>   
> -/* Measure the resolution of the clock so we can scale the number of
> -   benchmark iterations by this value.  */
> -# define TIMING_INIT(res) \
> -({                                                                           \
> -  struct timespec start;                                                     \
> -  clock_getres (CLOCK_PROCESS_CPUTIME_ID, &start);                           \
> -  (res) = start.tv_nsec;                                             \
> -})

OK.

> -
>   # define TIMING_NOW(var) \
>   ({                                                                            \
>     struct timespec tv;                                                        \
>      
>
diff mbox

Patch

diff --git a/benchtests/bench-malloc-simple.c b/benchtests/bench-malloc-simple.c
index 83203ff3187654a1710c9ef81016f854957b9d64..b8bb2cc116953c6691c17633d18c5661c7d9243e 100644
--- a/benchtests/bench-malloc-simple.c
+++ b/benchtests/bench-malloc-simple.c
@@ -86,9 +86,6 @@  bench (unsigned long size)
 {
   size_t iters = NUM_ITERS;
   int **arr = (int**) malloc (MAX_ALLOCS * sizeof (void*));
-  unsigned long res;
-
-  TIMING_INIT (res);
 
   for (int t = 0; t <= 3; t++)
     for (int i = 0; i < NUM_ALLOCS; i++)
diff --git a/benchtests/bench-malloc-thread.c b/benchtests/bench-malloc-thread.c
index bb4ba727a88059ecbe7305f5b8ad1693c1f1f266..52261425b0f1af32c17328ea5e0a5bb6f230df47 100644
--- a/benchtests/bench-malloc-thread.c
+++ b/benchtests/bench-malloc-thread.c
@@ -225,7 +225,6 @@  main (int argc, char **argv)
 {
   timing_t cur;
   size_t iters = 0, num_threads = 1;
-  unsigned long res;
   json_ctx_t json_ctx;
   double d_total_s, d_total_i;
   struct sigaction act;
@@ -261,10 +260,6 @@  main (int argc, char **argv)
 
   json_attr_object_begin (&json_ctx, "");
 
-  TIMING_INIT (res);
-
-  (void) res;
-
   memset (&act, 0, sizeof (act));
   act.sa_handler = &alarm_handler;
 
diff --git a/benchtests/bench-skeleton.c b/benchtests/bench-skeleton.c
index 37625c4296882268f6260d99adbc7f0295164ffc..854151e5a82028e74fe3a966e82004572542f411 100644
--- a/benchtests/bench-skeleton.c
+++ b/benchtests/bench-skeleton.c
@@ -48,14 +48,11 @@  main (int argc, char **argv)
 
   memset (&runtime, 0, sizeof (runtime));
 
-  unsigned long iters, res;
+  unsigned long iters = 1000;
 
 #ifdef BENCH_INIT
   BENCH_INIT ();
 #endif
-  TIMING_INIT (res);
-
-  iters = 1000 * res;
 
   json_init (&json_ctx, 2, stdout);
 
diff --git a/benchtests/bench-strtod.c b/benchtests/bench-strtod.c
index 4de0b9acb67eb925a80249322957ce8b3c08c8d6..d5b2503553ef74f33cace919ae9c62f79cd11c9c 100644
--- a/benchtests/bench-strtod.c
+++ b/benchtests/bench-strtod.c
@@ -89,9 +89,6 @@  int
 do_bench (void)
 {
   const size_t iters = INNER_LOOP_ITERS;
-  timing_t res __attribute__ ((unused));
-
-  TIMING_INIT (res);
 
   for (size_t i = 0; inputs[i] != NULL; ++i)
     {
diff --git a/benchtests/bench-timing.h b/benchtests/bench-timing.h
index 41b7324527b9deed67b3479cb1308fbd291bc5ca..f9b19fcd29efb45ea02c375e37caba94c93956d1 100644
--- a/benchtests/bench-timing.h
+++ b/benchtests/bench-timing.h
@@ -28,8 +28,6 @@  typedef hp_timing_t timing_t;
 
 # define TIMING_TYPE "hp_timing"
 
-# define TIMING_INIT(res) ({ (res) = 1; })
-
 # define TIMING_NOW(var) HP_TIMING_NOW (var)
 # define TIMING_DIFF(diff, start, end) HP_TIMING_DIFF ((diff), (start), (end))
 # define TIMING_ACCUM(sum, diff) HP_TIMING_ACCUM_NT ((sum), (diff))
@@ -41,15 +39,6 @@  typedef uint64_t timing_t;
 
 # define TIMING_TYPE "clock_gettime"
 
-/* Measure the resolution of the clock so we can scale the number of
-   benchmark iterations by this value.  */
-# define TIMING_INIT(res) \
-({                                                                           \
-  struct timespec start;                                                     \
-  clock_getres (CLOCK_PROCESS_CPUTIME_ID, &start);                           \
-  (res) = start.tv_nsec;                                             \
-})
-
 # define TIMING_NOW(var) \
 ({                                                                            \
   struct timespec tv;                                                        \