aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNamhyung Kim <namhyung@kernel.org>2014-04-21 22:44:21 -0400
committerJiri Olsa <jolsa@kernel.org>2014-04-24 10:32:15 -0400
commit820bc81f4cdaac09a8f25040d3a20d86f3da292b (patch)
treeafb1ff42a151339c55782af3b626078a81b21fd1
parent87e90f43285f4096e9ba5fc18b05c2e04caf3fab (diff)
perf tools: Account entry stats when it's added to the output tree
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>
-rw-r--r--tools/perf/builtin-annotate.c3
-rw-r--r--tools/perf/builtin-diff.c13
-rw-r--r--tools/perf/builtin-report.c24
-rw-r--r--tools/perf/util/hist.c1
4 files changed, 20 insertions, 21 deletions
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 0da603b79b61..d30d2c2e2a7a 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -46,7 +46,7 @@ struct perf_annotate {
46}; 46};
47 47
48static int perf_evsel__add_sample(struct perf_evsel *evsel, 48static int perf_evsel__add_sample(struct perf_evsel *evsel,
49 struct perf_sample *sample, 49 struct perf_sample *sample __maybe_unused,
50 struct addr_location *al, 50 struct addr_location *al,
51 struct perf_annotate *ann) 51 struct perf_annotate *ann)
52{ 52{
@@ -70,7 +70,6 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
70 return -ENOMEM; 70 return -ENOMEM;
71 71
72 ret = hist_entry__inc_addr_samples(he, evsel->idx, al->addr); 72 ret = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
73 evsel->hists.stats.total_period += sample->period;
74 hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE); 73 hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
75 return ret; 74 return ret;
76} 75}
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 52d91ac4e6c8..f3b10dcf6838 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -341,11 +341,16 @@ static int diff__process_sample_event(struct perf_tool *tool __maybe_unused,
341 return -1; 341 return -1;
342 } 342 }
343 343
344 if (al.filtered == 0) { 344 /*
345 evsel->hists.stats.total_non_filtered_period += sample->period; 345 * The total_period is updated here before going to the output
346 evsel->hists.nr_non_filtered_entries++; 346 * tree since normally only the baseline hists will call
347 } 347 * hists__output_resort() and precompute needs the total
348 * period in order to sort entries by percentage delta.
349 */
348 evsel->hists.stats.total_period += sample->period; 350 evsel->hists.stats.total_period += sample->period;
351 if (!al.filtered)
352 evsel->hists.stats.total_non_filtered_period += sample->period;
353
349 return 0; 354 return 0;
350} 355}
351 356
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index aed52036468d..89c95289fd51 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -85,6 +85,16 @@ static void report__inc_stats(struct report *rep, struct hist_entry *he)
85 */ 85 */
86 if (he->stat.nr_events == 1) 86 if (he->stat.nr_events == 1)
87 rep->nr_entries++; 87 rep->nr_entries++;
88
89 /*
90 * Only counts number of samples at this stage as it's more
91 * natural to do it here and non-sample events are also
92 * counted in perf_session_deliver_event(). The dump_trace
93 * requires this info is ready before going to the output tree.
94 */
95 hists__inc_nr_events(he->hists, PERF_RECORD_SAMPLE);
96 if (!he->filtered)
97 he->hists->stats.nr_non_filtered_samples++;
88} 98}
89 99
90static int report__add_mem_hist_entry(struct report *rep, struct addr_location *al, 100static int report__add_mem_hist_entry(struct report *rep, struct addr_location *al,
@@ -135,10 +145,6 @@ static int report__add_mem_hist_entry(struct report *rep, struct addr_location *
135 145
136 report__inc_stats(rep, he); 146 report__inc_stats(rep, he);
137 147
138 evsel->hists.stats.total_period += cost;
139 hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
140 if (!he->filtered)
141 evsel->hists.stats.nr_non_filtered_samples++;
142 err = hist_entry__append_callchain(he, sample); 148 err = hist_entry__append_callchain(he, sample);
143out: 149out:
144 return err; 150 return err;
@@ -189,13 +195,7 @@ static int report__add_branch_hist_entry(struct report *rep, struct addr_locatio
189 if (err) 195 if (err)
190 goto out; 196 goto out;
191 } 197 }
192
193 report__inc_stats(rep, he); 198 report__inc_stats(rep, he);
194
195 evsel->hists.stats.total_period += 1;
196 hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
197 if (!he->filtered)
198 evsel->hists.stats.nr_non_filtered_samples++;
199 } else 199 } else
200 goto out; 200 goto out;
201 } 201 }
@@ -230,10 +230,6 @@ static int report__add_hist_entry(struct report *rep, struct perf_evsel *evsel,
230 230
231 report__inc_stats(rep, he); 231 report__inc_stats(rep, he);
232 232
233 evsel->hists.stats.total_period += sample->period;
234 if (!he->filtered)
235 evsel->hists.stats.nr_non_filtered_samples++;
236 hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
237out: 233out:
238 return err; 234 return err;
239} 235}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 8d5cfcc3bc63..6d0d2d75db68 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -382,7 +382,6 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
382 if (!he) 382 if (!he)
383 return NULL; 383 return NULL;
384 384
385 hists->nr_entries++;
386 rb_link_node(&he->rb_node_in, parent, p); 385 rb_link_node(&he->rb_node_in, parent, p);
387 rb_insert_color(&he->rb_node_in, hists->entries_in); 386 rb_insert_color(&he->rb_node_in, hists->entries_in);
388out: 387out: