Commit-ID: baa2f6cedbfae962f04281a31f08ec29667d31a0
Gitweb: http://git.kernel.org/tip/baa2f6cedbfae962f04281a31f08ec29667d31a0
Author: Arnaldo Carvalho de Melo <[email protected]>
AuthorDate: Fri, 26 Nov 2010 19:39:15 -0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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)) &
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
<[email protected]> wrote:
> Commit-ID: baa2f6cedbfae962f04281a31f08ec29667d31a0
> Gitweb: http://git.kernel.org/tip/baa2f6cedbfae962f04281a31f08ec29667d31a0
> Author: Arnaldo Carvalho de Melo <[email protected]>
> AuthorDate: Fri, 26 Nov 2010 19:39:15 -0200
> Committer: Arnaldo Carvalho de Melo <[email protected]>
> 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 <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Mike Galbraith <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Stephane Eranian <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Tom Zanussi <[email protected]>
> LKML-Reference: <new-submission>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> ---
> 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)) &
>
Em Mon, Nov 29, 2010 at 11:22:50AM +0100, Stephane Eranian escreveu:
> 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.
Indeed, probably it is better to do that and only when doing a 'perf
archive' to try to compact the resulting perf.data.tar.bz2 file by
picking just the ones with hits.
I'll look into that, possibly today.
- Arnaldo