From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@kernel.org>,
Namhyung Kim <namhyung.kim@lge.com>,
LKML <linux-kernel@vger.kernel.org>, Jiri Olsa <jolsa@redhat.com>,
David Ahern <dsahern@gmail.com>,
Stephane Eranian <eranian@google.com>,
Andi Kleen <andi@firstfloor.org>,
Pekka Enberg <penberg@kernel.org>,
Frederic Weisbecker <fweisbec@gmail.com>
Subject: Re: [PATCH 2/3] perf tools: Record sampling time for each entry
Date: Mon, 2 Dec 2013 09:39:18 -0300 [thread overview]
Message-ID: <20131202123918.GE2371@ghostprotocols.net> (raw)
In-Reply-To: <1385967199-3759-3-git-send-email-namhyung@kernel.org>
Em Mon, Dec 02, 2013 at 03:53:18PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyung.kim@lge.com>
>
> Calculate elapsed time for each sample and record it. The elapsed
> time is a diff between current sample->time and previous sample->time
> which was saved for each evsel and cpu.
Can you elaborate on why this is interesting to have? What meaning can
one get from the sample->time - prev_sample->time?
We need clearer changelogs...
- Arnaldo
> Maybe we can use PERF_SAMPLE_READ for the precise result.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-annotate.c | 2 +-
> tools/perf/builtin-diff.c | 2 +-
> tools/perf/builtin-report.c | 9 +++++----
> tools/perf/builtin-top.c | 2 +-
> tools/perf/tests/hists_link.c | 4 ++--
> tools/perf/util/evsel.h | 1 +
> tools/perf/util/hist.c | 15 ++++++++++-----
> tools/perf/util/hist.h | 2 +-
> tools/perf/util/session.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/sort.h | 1 +
> 10 files changed, 65 insertions(+), 15 deletions(-)
>
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 4087ab19823c..dc43a64bf723 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -65,7 +65,7 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
> return 0;
> }
>
> - he = __hists__add_entry(&evsel->hists, al, NULL, NULL, NULL, 1, 1, 0);
> + he = __hists__add_entry(&evsel->hists, al, NULL, NULL, NULL, 1, 0, 1, 0);
> if (he == NULL)
> return -ENOMEM;
>
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index 3b67ea2444bd..85aa961a647e 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -307,7 +307,7 @@ static int hists__add_entry(struct hists *hists,
> struct addr_location *al, u64 period,
> u64 weight, u64 transaction)
> {
> - if (__hists__add_entry(hists, al, NULL, NULL, NULL, period, weight,
> + if (__hists__add_entry(hists, al, NULL, NULL, NULL, period, 0, weight,
> transaction) != NULL)
> return 0;
> return -ENOMEM;
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 1d47fbec4421..eb849e9f7093 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -116,7 +116,7 @@ static int perf_report__add_mem_hist_entry(struct perf_tool *tool,
> * and the he_stat__add_period() function.
> */
> he = __hists__add_entry(&evsel->hists, al, parent, NULL, mi,
> - cost, cost, 0);
> + cost, 0, cost, 0);
> if (!he)
> return -ENOMEM;
>
> @@ -210,7 +210,7 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
> * and not events sampled. Thus we use a pseudo period of 1.
> */
> he = __hists__add_entry(&evsel->hists, al, parent, &bi[i], NULL,
> - 1, 1, 0);
> + 1, 0, 1, 0);
> if (he) {
> struct annotation *notes;
> bx = he->branch_info;
> @@ -272,8 +272,9 @@ static int perf_evsel__add_hist_entry(struct perf_tool *tool,
> }
>
> he = __hists__add_entry(&evsel->hists, al, parent, NULL, NULL,
> - sample->period, sample->weight,
> - sample->transaction);
> + sample->period,
> + tool->ordered_samples ? sample->read.time_enabled : 0,
> + sample->weight, sample->transaction);
> if (he == NULL)
> return -ENOMEM;
>
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 03d37a76c612..175bde2a0ece 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -247,7 +247,7 @@ static struct hist_entry *perf_evsel__add_hist_entry(struct perf_evsel *evsel,
>
> pthread_mutex_lock(&evsel->hists.lock);
> he = __hists__add_entry(&evsel->hists, al, NULL, NULL, NULL,
> - sample->period, sample->weight,
> + sample->period, 0, sample->weight,
> sample->transaction);
> pthread_mutex_unlock(&evsel->hists.lock);
> if (he == NULL)
> diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
> index 173bf42cc03e..7bb952e6be62 100644
> --- a/tools/perf/tests/hists_link.c
> +++ b/tools/perf/tests/hists_link.c
> @@ -223,7 +223,7 @@ static int add_hist_entries(struct perf_evlist *evlist, struct machine *machine)
> goto out;
>
> he = __hists__add_entry(&evsel->hists, &al, NULL,
> - NULL, NULL, 1, 1, 0);
> + NULL, NULL, 1, 0, 1, 0);
> if (he == NULL)
> goto out;
>
> @@ -246,7 +246,7 @@ static int add_hist_entries(struct perf_evlist *evlist, struct machine *machine)
> goto out;
>
> he = __hists__add_entry(&evsel->hists, &al, NULL,
> - NULL, NULL, 1, 1, 0);
> + NULL, NULL, 1, 0, 1, 0);
> if (he == NULL)
> goto out;
>
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 20a7c653b74b..ac65fc67972c 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -69,6 +69,7 @@ struct perf_evsel {
> struct hists hists;
> u64 first_timestamp;
> u64 last_timestamp;
> + u64 *prev_timestamps;
> char *name;
> double scale;
> const char *unit;
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 822903eaa201..8d9278df59fa 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -204,11 +204,12 @@ static void hist_entry__add_cpumode_period(struct hist_entry *he,
> }
>
> static void he_stat__add_period(struct he_stat *he_stat, u64 period,
> - u64 weight)
> + u64 time, u64 weight)
> {
>
> he_stat->period += period;
> he_stat->weight += weight;
> + he_stat->time += time;
> he_stat->nr_events += 1;
> }
>
> @@ -221,10 +222,12 @@ static void he_stat__add_stat(struct he_stat *dest, struct he_stat *src)
> dest->period_guest_us += src->period_guest_us;
> dest->nr_events += src->nr_events;
> dest->weight += src->weight;
> + dest->time += src->time;
> }
>
> static void hist_entry__decay(struct hist_entry *he)
> {
> + he->stat.time = (he->stat.time * 7) / 8;
> he->stat.period = (he->stat.period * 7) / 8;
> he->stat.nr_events = (he->stat.nr_events * 7) / 8;
> /* XXX need decay for weight too? */
> @@ -344,7 +347,7 @@ static u8 symbol__parent_filter(const struct symbol *parent)
> static struct hist_entry *add_hist_entry(struct hists *hists,
> struct hist_entry *entry,
> struct addr_location *al,
> - u64 period,
> + u64 period, u64 time,
> u64 weight)
> {
> struct rb_node **p;
> @@ -367,7 +370,7 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
> cmp = hist_entry__cmp(he, entry);
>
> if (!cmp) {
> - he_stat__add_period(&he->stat, period, weight);
> + he_stat__add_period(&he->stat, period, time, weight);
>
> /*
> * This mem info was allocated from machine__resolve_mem
> @@ -412,7 +415,8 @@ struct hist_entry *__hists__add_entry(struct hists *hists,
> struct symbol *sym_parent,
> struct branch_info *bi,
> struct mem_info *mi,
> - u64 period, u64 weight, u64 transaction)
> + u64 period, u64 time,
> + u64 weight, u64 transaction)
> {
> struct hist_entry entry = {
> .thread = al->thread,
> @@ -428,6 +432,7 @@ struct hist_entry *__hists__add_entry(struct hists *hists,
> .nr_events = 1,
> .period = period,
> .weight = weight,
> + .time = time,
> },
> .parent = sym_parent,
> .filtered = symbol__parent_filter(sym_parent),
> @@ -437,7 +442,7 @@ struct hist_entry *__hists__add_entry(struct hists *hists,
> .transaction = transaction,
> };
>
> - return add_hist_entry(hists, &entry, al, period, weight);
> + return add_hist_entry(hists, &entry, al, period, time, weight);
> }
>
> int64_t
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index bc5acdfc2b4b..0433469812dc 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -89,7 +89,7 @@ struct hist_entry *__hists__add_entry(struct hists *hists,
> struct addr_location *al,
> struct symbol *parent,
> struct branch_info *bi,
> - struct mem_info *mi, u64 period,
> + struct mem_info *mi, u64 period, u64 time,
> u64 weight, u64 transaction);
> int64_t hist_entry__cmp(struct hist_entry *left, struct hist_entry *right);
> int64_t hist_entry__collapse(struct hist_entry *left, struct hist_entry *right);
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index e4b158f0586a..f36e95f00a05 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -926,6 +926,48 @@ perf_session__deliver_sample(struct perf_session *session,
> u64 read_format = evsel->attr.read_format;
>
> if (tool->ordered_samples) {
> + /* FIXME: need to find a way to determine cpu-wide session */
> + bool per_cpu_session = sample_type & PERF_SAMPLE_CPU;
> +
> + sample->read.time_enabled = 0;
> +
> + if (per_cpu_session) {
> + u64 *ts = evsel->prev_timestamps;
> +
> + if (ts == NULL) {
> + int nr_cpus = session->header.env.nr_cpus_online;
> +
> + ts = zalloc(nr_cpus * sizeof(ts));
> + if (ts == NULL)
> + return -ENOMEM;
> +
> + evsel->prev_timestamps = ts;
> + }
> +
> + if (ts[sample->cpu] != 0) {
> + u64 diff = sample->time - ts[sample->cpu];
> +
> + sample->read.time_enabled = diff;
> + }
> + ts[sample->cpu] = sample->time;
> + } else {
> + u64 *ts = evsel->prev_timestamps;
> +
> + if (ts == NULL) {
> + ts = zalloc(sizeof(*ts));
> + if (ts == NULL)
> + return -ENOMEM;
> +
> + evsel->prev_timestamps = ts;
> + }
> +
> + if (*ts != 0) {
> + u64 diff = sample->time - *ts;
> + sample->read.time_enabled = diff;
> + }
> + *ts = sample->time;
> + }
> +
> if (evsel->first_timestamp == 0)
> evsel->first_timestamp = sample->time;
>
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 43e5ff42a609..16aaf0a47346 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -51,6 +51,7 @@ struct he_stat {
> u64 period_guest_sys;
> u64 period_guest_us;
> u64 weight;
> + u64 time;
> u32 nr_events;
> };
>
> --
> 1.7.11.7
next prev parent reply other threads:[~2013-12-02 12:39 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-02 6:53 [RFC 0/3] perf tools: Show time info (v1) Namhyung Kim
2013-12-02 6:53 ` [PATCH 1/3] perf tools: Record total sampling time Namhyung Kim
2013-12-02 12:45 ` Ingo Molnar
2013-12-02 12:57 ` Ingo Molnar
2013-12-02 15:43 ` Namhyung Kim
2013-12-02 16:36 ` Ingo Molnar
2013-12-02 20:24 ` Arnaldo Carvalho de Melo
2013-12-03 5:44 ` Namhyung Kim
2013-12-03 14:30 ` David Ahern
2013-12-04 10:00 ` Ingo Molnar
2013-12-04 10:02 ` Ingo Molnar
2013-12-03 5:33 ` Namhyung Kim
2013-12-02 15:05 ` Namhyung Kim
2013-12-02 18:51 ` Arnaldo Carvalho de Melo
2013-12-02 6:53 ` [PATCH 2/3] perf tools: Record sampling time for each entry Namhyung Kim
2013-12-02 12:39 ` Arnaldo Carvalho de Melo [this message]
2013-12-02 14:57 ` Namhyung Kim
2013-12-02 18:49 ` Arnaldo Carvalho de Melo
2013-12-03 4:33 ` Namhyung Kim
2013-12-02 6:53 ` [PATCH 3/3] perf report: Add --show-time-info option Namhyung Kim
2013-12-02 12:33 ` Arnaldo Carvalho de Melo
2013-12-02 14:38 ` Namhyung Kim
2013-12-02 9:35 ` [RFC 0/3] perf tools: Show time info (v1) Pekka Enberg
2013-12-03 2:28 ` Namhyung Kim
2013-12-02 17:04 ` Andi Kleen
2013-12-03 2:34 ` Namhyung Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131202123918.GE2371@ghostprotocols.net \
--to=acme@ghostprotocols.net \
--cc=a.p.zijlstra@chello.nl \
--cc=andi@firstfloor.org \
--cc=dsahern@gmail.com \
--cc=eranian@google.com \
--cc=fweisbec@gmail.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung.kim@lge.com \
--cc=namhyung@kernel.org \
--cc=paulus@samba.org \
--cc=penberg@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.