Re: [PATCH v1 01/10] perf bench evlist-open-close: Reduce scope of 2 variables

[Arnaldo Carvalho de Melo]

On Sat, Jan 11, 2025 at 11:01:34AM -0800, Ian Rogers wrote:
> Make 2 global variables local. Reduces ELF binary size by removing
> relocations. For a no flags build, the perf binary size is reduced by
> 4,144 bytes on x86-64.

I'm trying to reproduce your results:

  $ gcc --version | head -1
  gcc (GCC) 14.2.1 20240912 (Red Hat 14.2.1-3)
  $
  $ rm -rf /tmp/build/$(basename $PWD)/ ; mkdir -p /tmp/build/$(basename $PWD)/  
  $ make -k O=/tmp/build/$(basename $PWD)/ -C tools/perf install-bin

Without your patch:

  $ ls -la ~/bin/perf
  -rwxr-xr-x. 2 acme acme 11411680 Feb 12 10:57 /home/acme/bin/perf
  $ size ~/bin/perf
      text     data     bss       dec     hex  filename
  10071297   302496   34540  10408333  9ed18d  /home/acme/bin/perf
  $

Then, with your patch:

  $ git log --oneline -1
  abd904389b3f0807 (HEAD -> perf-tools-next) perf bench evlist-open-close: Reduce scope of 2 variables
  $ perf -v
  perf version 6.13.rc2.gabd904389b3f
  $ size ~/bin/perf
      text     data     bss       dec     hex  filename
  10072001   301568   34540  10408109  9ed0ad  /home/acme/bin/perf
  $
  $ ls -la ~/bin/perf
  -rwxr-xr-x. 2 acme acme 11411632 Feb 12 11:02 /home/acme/bin/perf
  $

So a more modest 224 bytes reduction in the perf binary size.

In the distant past several of these moves from global to local were
made, for instance:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d20deb64e0490ee9442b5181bc08a62d2cadcb90

I tried but didn't find the before/after effects on binary size...

Anyways,

Reviewed-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
Tested-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>

Thanks,

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
>  tools/perf/bench/evlist-open-close.c | 42 +++++++++++++++-------------
>  1 file changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/perf/bench/evlist-open-close.c b/tools/perf/bench/evlist-open-close.c
> index 5a27691469ed..79cedcf94a39 100644
> --- a/tools/perf/bench/evlist-open-close.c
> +++ b/tools/perf/bench/evlist-open-close.c
> @@ -46,25 +46,6 @@ static struct record_opts opts = {
>  	.ctl_fd_ack          = -1,
>  };
>  
> -static const struct option options[] = {
> -	OPT_STRING('e', "event", &event_string, "event", "event selector. use 'perf list' to list available events"),
> -	OPT_INTEGER('n', "nr-events", &nr_events,
> -		     "number of dummy events to create (default 1). If used with -e, it clones those events n times (1 = no change)"),
> -	OPT_INTEGER('i', "iterations", &iterations, "Number of iterations used to compute average (default=100)"),
> -	OPT_BOOLEAN('a', "all-cpus", &opts.target.system_wide, "system-wide collection from all CPUs"),
> -	OPT_STRING('C', "cpu", &opts.target.cpu_list, "cpu", "list of cpus where to open events"),
> -	OPT_STRING('p', "pid", &opts.target.pid, "pid", "record events on existing process id"),
> -	OPT_STRING('t', "tid", &opts.target.tid, "tid", "record events on existing thread id"),
> -	OPT_STRING('u', "uid", &opts.target.uid_str, "user", "user to profile"),
> -	OPT_BOOLEAN(0, "per-thread", &opts.target.per_thread, "use per-thread mmaps"),
> -	OPT_END()
> -};
> -
> -static const char *const bench_usage[] = {
> -	"perf bench internals evlist-open-close <options>",
> -	NULL
> -};
> -
>  static int evlist__count_evsel_fds(struct evlist *evlist)
>  {
>  	struct evsel *evsel;
> @@ -225,6 +206,29 @@ static char *bench__repeat_event_string(const char *evstr, int n)
>  
>  int bench_evlist_open_close(int argc, const char **argv)
>  {
> +	const struct option options[] = {
> +		OPT_STRING('e', "event", &event_string, "event",
> +			   "event selector. use 'perf list' to list available events"),
> +		OPT_INTEGER('n', "nr-events", &nr_events,
> +			    "number of dummy events to create (default 1). If used with -e, it clones those events n times (1 = no change)"),
> +		OPT_INTEGER('i', "iterations", &iterations,
> +			    "Number of iterations used to compute average (default=100)"),
> +		OPT_BOOLEAN('a', "all-cpus", &opts.target.system_wide,
> +			    "system-wide collection from all CPUs"),
> +		OPT_STRING('C', "cpu", &opts.target.cpu_list, "cpu",
> +			   "list of cpus where to open events"),
> +		OPT_STRING('p', "pid", &opts.target.pid, "pid",
> +			   "record events on existing process id"),
> +		OPT_STRING('t', "tid", &opts.target.tid, "tid",
> +			   "record events on existing thread id"),
> +		OPT_STRING('u', "uid", &opts.target.uid_str, "user", "user to profile"),
> +		OPT_BOOLEAN(0, "per-thread", &opts.target.per_thread, "use per-thread mmaps"),
> +		OPT_END()
> +	};
> +	const char *const bench_usage[] = {
> +		"perf bench internals evlist-open-close <options>",
> +		NULL
> +	};
>  	char *evstr, errbuf[BUFSIZ];
>  	int err;
>  
> -- 
> 2.47.1.613.gc27f4b7a9f-goog
>