Skip to content

Commit

Permalink
perf tools: Account entry stats when it's added to the output tree
Browse files Browse the repository at this point in the history
Currently, accounting each sample is done in multiple places - once
when adding them to the input tree, other when adding them to the
output tree.  It's not only confusing but also can cause a subtle
problem since concurrent processing like in perf top might see the
updated stats before adding entries into the output tree - like seeing
more (blank) lines at the end and/or slight inaccurate percentage.

To fix this, only account the entries when it's moved into the output
tree so that they cannot be seen prematurely.  There're some
exceptional cases here and there - they should be addressed separately
with comments.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1398327843-31845-7-git-send-email-namhyung@kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
  • Loading branch information
Namhyung Kim authored and Jiri Olsa committed Apr 24, 2014
1 parent 87e90f4 commit 820bc81
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 21 deletions.
3 changes: 1 addition & 2 deletions tools/perf/builtin-annotate.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ struct perf_annotate {
};

static int perf_evsel__add_sample(struct perf_evsel *evsel,
struct perf_sample *sample,
struct perf_sample *sample __maybe_unused,
struct addr_location *al,
struct perf_annotate *ann)
{
Expand All @@ -70,7 +70,6 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
return -ENOMEM;

ret = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
evsel->hists.stats.total_period += sample->period;
hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
return ret;
}
Expand Down
13 changes: 9 additions & 4 deletions tools/perf/builtin-diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -341,11 +341,16 @@ static int diff__process_sample_event(struct perf_tool *tool __maybe_unused,
return -1;
}

if (al.filtered == 0) {
evsel->hists.stats.total_non_filtered_period += sample->period;
evsel->hists.nr_non_filtered_entries++;
}
/*
* The total_period is updated here before going to the output
* tree since normally only the baseline hists will call
* hists__output_resort() and precompute needs the total
* period in order to sort entries by percentage delta.
*/
evsel->hists.stats.total_period += sample->period;
if (!al.filtered)
evsel->hists.stats.total_non_filtered_period += sample->period;

return 0;
}

Expand Down
24 changes: 10 additions & 14 deletions tools/perf/builtin-report.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ static void report__inc_stats(struct report *rep, struct hist_entry *he)
*/
if (he->stat.nr_events == 1)
rep->nr_entries++;

/*
* Only counts number of samples at this stage as it's more
* natural to do it here and non-sample events are also
* counted in perf_session_deliver_event(). The dump_trace
* requires this info is ready before going to the output tree.
*/
hists__inc_nr_events(he->hists, PERF_RECORD_SAMPLE);
if (!he->filtered)
he->hists->stats.nr_non_filtered_samples++;
}

static int report__add_mem_hist_entry(struct report *rep, struct addr_location *al,
Expand Down Expand Up @@ -135,10 +145,6 @@ static int report__add_mem_hist_entry(struct report *rep, struct addr_location *

report__inc_stats(rep, he);

evsel->hists.stats.total_period += cost;
hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
if (!he->filtered)
evsel->hists.stats.nr_non_filtered_samples++;
err = hist_entry__append_callchain(he, sample);
out:
return err;
Expand Down Expand Up @@ -189,13 +195,7 @@ static int report__add_branch_hist_entry(struct report *rep, struct addr_locatio
if (err)
goto out;
}

report__inc_stats(rep, he);

evsel->hists.stats.total_period += 1;
hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
if (!he->filtered)
evsel->hists.stats.nr_non_filtered_samples++;
} else
goto out;
}
Expand Down Expand Up @@ -230,10 +230,6 @@ static int report__add_hist_entry(struct report *rep, struct perf_evsel *evsel,

report__inc_stats(rep, he);

evsel->hists.stats.total_period += sample->period;
if (!he->filtered)
evsel->hists.stats.nr_non_filtered_samples++;
hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
out:
return err;
}
Expand Down
1 change: 0 additions & 1 deletion tools/perf/util/hist.c
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,6 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
if (!he)
return NULL;

hists->nr_entries++;
rb_link_node(&he->rb_node_in, parent, p);
rb_insert_color(&he->rb_node_in, hists->entries_in);
out:
Expand Down

0 comments on commit 820bc81

Please sign in to comment.