Re: [PATCH] perf record: Add record.build-id config option

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Tue, Dec 15, 2015 at 10:49:56AM +0900, Namhyung Kim escreveu:
> Post processing at perf record takes long time on big machines.  What it
> does is to find build-id of related binaries.  Sometimes we just want to
> skip the processing and get the result quickly.  Add a new config option
> to control this behavior.

Applied, after changing the above comment to:

-------------------------------------------------------------------

Post processing at 'perf record' takes a long time on big machines.

What it does is to find the build-id of binaries found in the event
stream, so that it can make sure, at report time, that the symtabs (Be
it ELF, kallsyms, etc) being used to resolve symbols are the ones
matching the binaries found at record time.

Sometimes we just want to skip this processing of events at the end of
the session to get quicker results, making sure the binaries haven't
changed from 'record' to 'report' time.

-------------------------------------------------------------------

The comments on the documentation already stressed that, but I improved
that a bit as well

 
> The record.build-id config variable can have one of following:
> 
>  - cache: post-process data and save/update the binaries into the
>           build-id cache (in ~/.debug).  This is default.
>  - no-cache: post-process data but not update the build-id cache.
>              Same effect with using -N option.
>  - skip: skip post-processing and not update the cache.
>          Same effect with using -B option.
> 
> Reported-by: Peter Zijlstra <peterz@infradead.org>
> Cc: Taeung Song <treeze.taeung@gmail.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/Documentation/perf-record.txt | 11 ++++++++++-
>  tools/perf/builtin-record.c              | 13 +++++++++++++
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index e630a7d2c348..809ac8701808 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -207,11 +207,20 @@ comma-separated list with no space: 0,1. Ranges of CPUs are specified with -: 0-
>  In per-thread mode with inheritance mode on (default), samples are captured only when
>  the thread executes on the designated CPUs. Default is to monitor all CPUs.
>  
> +-B::
> +--no-buildid::
> +Do not save the buildid of (used) binaries in the data file. This skips
> +post-processing after recoring which might take some time on big machines.
> +The Downside is that it can resolve into a wrong symbol after a binary is
> +rebuilt later.  You can also set "record.build-id" config variable to
> +'skip' to have same effect.

Do not save the build ids of binaries in the perf.data files. This skips
post processing after recording, which sometimes makes the final step in
the recording process to take a long time, as it needs to process all
events looking for mmap records. The downside is that it can misresolve
symbols if the workload binaries used when recording get locally rebuilt
or upgraded, because the only key available in this case is the
pathname. You can also set the "record.build-id" config variable to
'skip to have this behaviour permanently.

> +
>  -N::
>  --no-buildid-cache::
>  Do not update the buildid cache. This saves some overhead in situations
>  where the information in the perf.data file (which includes buildids)
> -is sufficient.
> +is sufficient.  You can also set "record.build-id" config variable to
> +'no-cache' to have same effect.
                                   the
                     the
 
>  
>  -G name,...::
>  --cgroup name,...::
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 199fc31e3919..424de323c1f8 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -837,6 +837,19 @@ int record_callchain_opt(const struct option *opt,
>  
>  static int perf_record_config(const char *var, const char *value, void *cb)
>  {
> +	struct record *rec = cb;
> +
> +	if (!strcmp(var, "record.build-id")) {
> +		if (!strcmp(value, "cache"))
> +			rec->no_buildid_cache = false;
> +		else if (!strcmp(value, "no-cache"))
> +			rec->no_buildid_cache = true;
> +		else if (!strcmp(value, "skip"))
> +			rec->no_buildid = true;
> +		else
> +			return -1;
> +		return 0;
> +	}
>  	if (!strcmp(var, "record.call-graph"))
>  		var = "call-graph.record-mode"; /* fall-through */
>  
> -- 
> 2.6.4