Re: [PATCH 6/7] perf bench: Run inject-build-id with --buildid-all option too

From: Ian Rogers <irogers@google.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Jiri Olsa <jolsa@redhat.com>, Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 6/7] perf bench: Run inject-build-id with --buildid-all option too
Date: Wed, 23 Sep 2020 15:17:29 -0700	[thread overview]
Message-ID: <CAP-5=fUqkcKS-THxNEz1K4irb-h9qsF09eLbTWfNKkV5StEuRw@mail.gmail.com> (raw)
In-Reply-To: <20200923080537.155264-7-namhyung@kernel.org>

On Wed, Sep 23, 2020 at 1:06 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> For comparison, it now runs the benchmark twice - one if regular -b
> and another for --buildid-all.
>
>   $ perf bench internals inject-build-id
>   # Running 'internals/inject-build-id' benchmark:
>     Average build-id injection took: 18.441 msec (+- 0.106 msec)
>     Average time per event: 1.808 usec (+- 0.010 usec)
>     Average build-id-all injection took: 13.451 msec (+- 0.132 msec)
>     Average time per event: 1.319 usec (+- 0.013 usec)
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/bench/inject-buildid.c | 47 ++++++++++++++++++++-----------
>  1 file changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/bench/inject-buildid.c b/tools/perf/bench/inject-buildid.c
> index e5144a85d689..7c9f2baecef2 100644
> --- a/tools/perf/bench/inject-buildid.c
> +++ b/tools/perf/bench/inject-buildid.c
> @@ -220,7 +220,7 @@ static void sigpipe_handler(int sig __maybe_unused)
>         /* child exited */
>  }
>
> -static int setup_injection(struct bench_data *data)
> +static int setup_injection(struct bench_data *data, bool build_id_all)
>  {
>         int ready_pipe[2];
>         int dev_null_fd;
> @@ -241,6 +241,7 @@ static int setup_injection(struct bench_data *data)
>
>         if (data->pid == 0) {
>                 const char **inject_argv;
> +               int inject_argc = 2;
>
>                 close(data->input_pipe[1]);
>                 close(data->output_pipe[0]);
> @@ -257,17 +258,22 @@ static int setup_injection(struct bench_data *data)
>
>                 dup2(dev_null_fd, STDERR_FILENO);
>
> -               inject_argv = calloc(3, sizeof(*inject_argv));
> +               if (build_id_all)
> +                       inject_argc++;
> +
> +               inject_argv = calloc(inject_argc + 1, sizeof(*inject_argv));
>                 if (inject_argv == NULL)
>                         exit(1);
>
>                 inject_argv[0] = strdup("inject");
>                 inject_argv[1] = strdup("-b");
> +               if (build_id_all)
> +                       inject_argv[2] = strdup("--buildid-all");
>
>                 /* signal that we're ready to go */
>                 close(ready_pipe[1]);
>
> -               cmd_inject(2, inject_argv);
> +               cmd_inject(inject_argc, inject_argv);
>
>                 exit(0);
>         }
> @@ -348,21 +354,14 @@ static int inject_build_id(struct bench_data *data)
>         return 0;
>  }
>
> -static int do_inject_loop(struct bench_data *data)
> +static void do_inject_loop(struct bench_data *data, bool build_id_all)
>  {
>         unsigned int i;
>         struct stats time_stats;
>         double time_average, time_stddev;
>
> -       srand(time(NULL));
>         init_stats(&time_stats);
> -       symbol__init(NULL);
> -
> -       collect_dso();
> -       if (nr_dsos == 0) {
> -               printf("  Cannot collect DSOs for injection\n");
> -               return -1;
> -       }
> +       pr_debug("  Build-id%s injection benchmark\n", build_id_all ? "-all" : "");
>
>         for (i = 0; i < iterations; i++) {
>                 struct timeval start, end, diff;
> @@ -370,7 +369,7 @@ static int do_inject_loop(struct bench_data *data)
>
>                 pr_debug("  Iteration #%d\n", i+1);
>
> -               if (setup_injection(data) < 0) {
> +               if (setup_injection(data, build_id_all) < 0) {
>                         printf("  Build-id injection setup failed\n");
>                         break;
>                 }
> @@ -389,14 +388,30 @@ static int do_inject_loop(struct bench_data *data)
>
>         time_average = avg_stats(&time_stats) / USEC_PER_MSEC;
>         time_stddev = stddev_stats(&time_stats) / USEC_PER_MSEC;
> -       printf("  Average build-id injection took: %.3f msec (+- %.3f msec)\n",
> -               time_average, time_stddev);
> +       printf("  Average build-id%s injection took: %.3f msec (+- %.3f msec)\n",
> +              build_id_all ? "-all" : "", time_average, time_stddev);
>
>         /* each iteration, it processes MMAP2 + BUILD_ID + nr_samples * SAMPLE */
>         time_average = avg_stats(&time_stats) / (nr_mmaps * (nr_samples + 2));
>         time_stddev = stddev_stats(&time_stats) / (nr_mmaps * (nr_samples + 2));
>         printf("  Average time per event: %.3f usec (+- %.3f usec)\n",
>                 time_average, time_stddev);
> +}
> +
> +static int do_inject_loops(struct bench_data *data)
> +{
> +
> +       srand(time(NULL));
> +       symbol__init(NULL);
> +
> +       collect_dso();
> +       if (nr_dsos == 0) {
> +               printf("  Cannot collect DSOs for injection\n");
> +               return -1;
> +       }
> +
> +       do_inject_loop(data, false);
> +       do_inject_loop(data, true);
>
>         release_dso();
>         return 0;
> @@ -412,6 +427,6 @@ int bench_inject_build_id(int argc, const char **argv)
>                 exit(EXIT_FAILURE);
>         }
>
> -       return do_inject_loop(&data);
> +       return do_inject_loops(&data);
>  }
>
> --
> 2.28.0.681.g6f77f65b4e-goog
>