Re: [tip:perf/core] perf record: Add option to disable collecting build-ids

[Stephane Eranian]

Arnaldo,

Indeed, collecting buildids at the end of a perf record session
is a very time AND memory consuming phase. I have seen
system oom because of this when running inside cgroup with
low memory.

This is easy to reproduce running: perf record -a -- ./Run shell.
With this, you see perf record reaching a RSS first plateau during
the active collection of the samples, i.e., dumping the kernel
buffer on disk. But then, when it calls process_buildids(), it shoots
way up in memory consumption. For instance, I have seen a
perf record running at 10MB RSS shooting all the way to 250MB
RSS during that phase. At first, I thought there was a memory
leak somewhere. But after instrumenting for a while, nothing
really showed up.

I think the problem for RSS is not so much reloading the entire
buffer, but rather that you are recreating the entire addresses spaces
of all processes captured. The reason: you only want to save the
buildids of the DSO for which you had at least one sample. Thus,
you have to allocate/de-allocate tons of threads and map structures.
I wonder if simply looking for MMAP samples and storing the buildids
(even if they have no samples) wouldn't be more efficient in some
cases. I believe it would be faster and less memory greedy.


On Sun, Nov 28, 2010 at 9:33 AM, tip-bot for Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
> Commit-ID: Âbaa2f6cedbfae962f04281a31f08ec29667d31a0
> Gitweb: Â Â http://git.kernel.org/tip/baa2f6cedbfae962f04281a31f08ec29667d31a0
> Author: Â Â Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> AuthorDate: Fri, 26 Nov 2010 19:39:15 -0200
> Committer: ÂArnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> CommitDate: Fri, 26 Nov 2010 19:39:15 -0200
>
> perf record: Add option to disable collecting build-ids
>
> Collecting build-ids for long running sessions may take a long time
> because it needs to traverse the whole just collected perf.data stream
> of events, marking the DSOs that had hits and then looking for the
> .note.gnu.build-id ELF section.
>
> For things like the 'trace' tool that records and right away consumes
> the data on systems where its unlikely that the DSOs being monitored
> will change while 'trace' runs, it is desirable to remove build id
> collection, so add a -B/--no-buildid option to perf record to allow such
> use case.
>
> Longer term we'll avoid all this if we, at DSO load time, in the kernel,
> take advantage of this slow code path to collect the build-id and stash
> it somewhere, so that we can insert it in the PERF_RECORD_MMAP event.
>
> Reported-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> Cc: Mike Galbraith <efault@xxxxxx>
> Cc: Paul Mackerras <paulus@xxxxxxxxx>
> Cc: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> Cc: Stephane Eranian <eranian@xxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Tom Zanussi <tzanussi@xxxxxxxxx>
> LKML-Reference: <new-submission>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> ---
> Âtools/perf/builtin-record.c      Â|  14 +++++++++++---
> Âtools/perf/util/header.c        |  11 +++++++++--
> Âtools/perf/util/header.h        |  Â1 +
> Âtools/perf/util/include/linux/bitops.h | Â Â5 +++++
> Â4 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 3d2cb48..024e144 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -61,6 +61,7 @@ static bool          inherit_stat          Â= Âfalse;
> Âstatic bool          Âno_samples           Â= Âfalse;
> Âstatic bool          Âsample_address         Â= Âfalse;
> Âstatic bool          Âno_buildid           Â= Âfalse;
> +static bool          Âno_buildid_cache        Â= Âfalse;
>
> Âstatic long          Âsamples             =   Â0;
> Âstatic u64           bytes_written          =   Â0;
> @@ -437,7 +438,8 @@ static void atexit_header(void)
> Â Â Â Âif (!pipe_output) {
> Â Â Â Â Â Â Â Âsession->header.data_size += bytes_written;
>
> - Â Â Â Â Â Â Â process_buildids();
> + Â Â Â Â Â Â Â if (!no_buildid)
> + Â Â Â Â Â Â Â Â Â Â Â process_buildids();
> Â Â Â Â Â Â Â Âperf_header__write(&session->header, output, true);
> Â Â Â Â Â Â Â Âperf_session__delete(session);
> Â Â Â Â Â Â Â Âsymbol__exit();
> @@ -557,6 +559,9 @@ static int __cmd_record(int argc, const char **argv)
> Â Â Â Â Â Â Â Âreturn -1;
> Â Â Â Â}
>
> + Â Â Â if (!no_buildid)
> + Â Â Â Â Â Â Â perf_header__set_feat(&session->header, HEADER_BUILD_ID);
> +
> Â Â Â Âif (!file_new) {
> Â Â Â Â Â Â Â Âerr = perf_header__read(session, output);
> Â Â Â Â Â Â Â Âif (err < 0)
> @@ -831,8 +836,10 @@ const struct option record_options[] = {
> Â Â Â Â Â Â Â Â Â Â"Sample addresses"),
> Â Â Â ÂOPT_BOOLEAN('n', "no-samples", &no_samples,
> Â Â Â Â Â Â Â Â Â Â"don't sample"),
> - Â Â Â OPT_BOOLEAN('N', "no-buildid-cache", &no_buildid,
> + Â Â Â OPT_BOOLEAN('N', "no-buildid-cache", &no_buildid_cache,
> Â Â Â Â Â Â Â Â Â Â"do not update the buildid cache"),
> + Â Â Â OPT_BOOLEAN('B', "no-buildid", &no_buildid,
> + Â Â Â Â Â Â Â Â Â "do not collect buildids in perf.data"),
> Â Â Â ÂOPT_END()
> Â};
>
> @@ -857,7 +864,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
> Â Â Â Â}
>
> Â Â Â Âsymbol__init();
> - Â Â Â if (no_buildid)
> +
> + Â Â Â if (no_buildid_cache || no_buildid)
> Â Â Â Â Â Â Â Âdisable_buildid_cache();
>
> Â Â Â Âif (!nr_counters) {
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index d7e67b1..f65d7dc 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -152,6 +152,11 @@ void perf_header__set_feat(struct perf_header *self, int feat)
> Â Â Â Âset_bit(feat, self->adds_features);
> Â}
>
> +void perf_header__clear_feat(struct perf_header *self, int feat)
> +{
> + Â Â Â clear_bit(feat, self->adds_features);
> +}
> +
> Âbool perf_header__has_feat(const struct perf_header *self, int feat)
> Â{
> Â Â Â Âreturn test_bit(feat, self->adds_features);
> @@ -431,8 +436,10 @@ static int perf_header__adds_write(struct perf_header *self, int fd)
> Â Â Â Âint idx = 0, err;
>
> Â Â Â Âsession = container_of(self, struct perf_session, header);
> - Â Â Â if (perf_session__read_build_ids(session, true))
> - Â Â Â Â Â Â Â perf_header__set_feat(self, HEADER_BUILD_ID);
> +
> + Â Â Â if (perf_header__has_feat(self, HEADER_BUILD_ID &&
> + Â Â Â Â Â !perf_session__read_build_ids(session, true)))
> + Â Â Â Â Â Â Â perf_header__clear_feat(self, HEADER_BUILD_ID);
>
> Â Â Â Ânr_sections = bitmap_weight(self->adds_features, HEADER_FEAT_BITS);
> Â Â Â Âif (!nr_sections)
> diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
> index 402ac24..ed550bf 100644
> --- a/tools/perf/util/header.h
> +++ b/tools/perf/util/header.h
> @@ -84,6 +84,7 @@ u64 perf_header__sample_type(struct perf_header *header);
> Âstruct perf_event_attr *
> Âperf_header__find_attr(u64 id, struct perf_header *header);
> Âvoid perf_header__set_feat(struct perf_header *self, int feat);
> +void perf_header__clear_feat(struct perf_header *self, int feat);
> Âbool perf_header__has_feat(const struct perf_header *self, int feat);
>
> Âint perf_header__process_sections(struct perf_header *self, int fd,
> diff --git a/tools/perf/util/include/linux/bitops.h b/tools/perf/util/include/linux/bitops.h
> index bb4ac2e..8be0b96 100644
> --- a/tools/perf/util/include/linux/bitops.h
> +++ b/tools/perf/util/include/linux/bitops.h
> @@ -13,6 +13,11 @@ static inline void set_bit(int nr, unsigned long *addr)
> Â Â Â Âaddr[nr / BITS_PER_LONG] |= 1UL << (nr % BITS_PER_LONG);
> Â}
>
> +static inline void clear_bit(int nr, unsigned long *addr)
> +{
> + Â Â Â addr[nr / BITS_PER_LONG] &= ~(1UL << (nr % BITS_PER_LONG));
> +}
> +
> Âstatic __always_inline int test_bit(unsigned int nr, const unsigned long *addr)
> Â{
> Â Â Â Âreturn ((1UL << (nr % BITS_PER_LONG)) &
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/