Re: [PATCH v4 1/3] perf report: compile tips.txt in perf binary

[Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco)]

> I dont mind to keep static count of tips, but would be great
> to have some check when there are more tips in the tips.txt
>
> or perhaps pick tip with random index within the tips data size?
> you would just do strtok until you reach the string that has the
> random index inside.. you wouldn't need str pointers then

There is no static counter of tips. MAX_TIPS is just maximum number
of tips. Number of tips in tips.txt file:

tools/perf$ wc -l Documentation/tips.txt
43 Documentation/tips.txt

By invoking strtok(..."\n") in a while loop we count how many tips do we have
and also save a pointer to corresponding tip in str[]
After we randomly pick up one of those tips (how it works previously).

________________________________________
From: Jiri Olsa <jolsa@xxxxxxxxxx>
Sent: 06 May 2021 16:54
To: Denys Zagorui -X (dzagorui - GLOBALLOGIC INC at Cisco)
Cc: linux-kernel@xxxxxxxxxxxxxxx; peterz@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; acme@xxxxxxxxxx; mark.rutland@xxxxxxx; alexander.shishkin@xxxxxxxxxxxxxxx; namhyung@xxxxxxxxxx
Subject: Re: [PATCH v4 1/3] perf report: compile tips.txt in perf binary

On Fri, Apr 30, 2021 at 06:33:48AM -0700, Denys Zagorui wrote:
> It seems there is some need to have an ability to invoke perf from
> build directory without installation
> (84cfac7f05e1: perf tools: Set and pass DOCDIR to builtin-report.c)
> DOCDIR definition contains an absolute path to kernel source directory.
> It is build machine related info and it makes perf binary unreproducible.
>
> This can be avoided by compiling tips.txt in perf directly.
>
> Signed-off-by: Denys Zagorui <dzagorui@xxxxxxxxx>
> ---
>  tools/perf/Build               |  2 +-
>  tools/perf/Documentation/Build |  9 ++++++++
>  tools/perf/builtin-report.c    | 39 ++++++++++++++++++++++++++--------
>  tools/perf/util/util.c         | 28 ------------------------
>  tools/perf/util/util.h         |  2 --
>  5 files changed, 40 insertions(+), 40 deletions(-)
>  create mode 100644 tools/perf/Documentation/Build
>
> diff --git a/tools/perf/Build b/tools/perf/Build
> index db61dbe2b543..3a2e768d7576 100644
> --- a/tools/perf/Build
> +++ b/tools/perf/Build
> @@ -45,12 +45,12 @@ CFLAGS_perf.o              += -DPERF_HTML_PATH="BUILD_STR($(htmldir_SQ))" \
>                             -DPREFIX="BUILD_STR($(prefix_SQ))"
>  CFLAGS_builtin-trace.o          += -DSTRACE_GROUPS_DIR="BUILD_STR($(STRACE_GROUPS_DIR_SQ))"
>  CFLAGS_builtin-report.o         += -DTIPDIR="BUILD_STR($(tipdir_SQ))"
> -CFLAGS_builtin-report.o         += -DDOCDIR="BUILD_STR($(srcdir_SQ)/Documentation)"
>
>  perf-y += util/
>  perf-y += arch/
>  perf-y += ui/
>  perf-y += scripts/
>  perf-$(CONFIG_TRACE) += trace/beauty/
> +perf-y += Documentation/
>
>  gtk-y += ui/gtk/
> diff --git a/tools/perf/Documentation/Build b/tools/perf/Documentation/Build
> new file mode 100644
> index 000000000000..83e16764caa4
> --- /dev/null
> +++ b/tools/perf/Documentation/Build
> @@ -0,0 +1,9 @@
> +perf-y += tips.o
> +
> +quiet_cmd_ld_tips = LD       $@
> +      cmd_ld_tips = $(LD) -r -b binary -o $@ $<
> +
> +$(OUTPUT)Documentation/tips.o: Documentation/tips.txt FORCE
> +     $(call rule_mkdir)
> +     $(call if_changed,ld_tips)

nice, I had no idea ld could do it like this ;-)

> +
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 2a845d6cac09..88375ed76d53 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -47,7 +47,6 @@
>  #include "util/time-utils.h"
>  #include "util/auxtrace.h"
>  #include "util/units.h"
> -#include "util/util.h" // perf_tip()
>  #include "ui/ui.h"
>  #include "ui/progress.h"
>  #include "util/block-info.h"
> @@ -107,6 +106,9 @@ struct report {
>       int                     nr_block_reports;
>  };
>
> +extern char _binary_Documentation_tips_txt_start[];
> +extern char _binary_Documentation_tips_txt_end[];
> +
>  static int report__config(const char *var, const char *value, void *cb)
>  {
>       struct report *rep = cb;
> @@ -604,19 +606,38 @@ static int report__gtk_browse_hists(struct report *rep, const char *help)
>       return hist_browser(rep->session->evlist, help, NULL, rep->min_percent);
>  }
>
> +#define MAX_TIPS        60
> +
> +static const char *perf_tip(void)
> +{
> +     char *str[MAX_TIPS];
> +     int i = 0;
> +
> +     _binary_Documentation_tips_txt_start[_binary_Documentation_tips_txt_end -
> +             _binary_Documentation_tips_txt_start - 1] = 0;
> +
> +     str[i] = strtok(_binary_Documentation_tips_txt_start, "\n");
> +     if (!str[i])
> +             return "Tips cannot be found!";
> +
> +     i++;
> +
> +     while (i < MAX_TIPS) {
> +             str[i] = strtok(NULL, "\n");
> +             if (!str[i])
> +                     break;
> +             i++;
> +     }

I dont mind to keep static count of tips, but would be great
to have some check when there are more tips in the tips.txt

or perhaps pick tip with random index within the tips data size?
you would just do strtok until you reach the string that has the
random index inside.. you wouldn't need str pointers then

jirka