diff options
author | Jin Yao <yao.jin@linux.intel.com> | 2019-03-15 17:16:17 -0400 |
---|---|---|
committer | Arnaldo Carvalho de Melo <acme@redhat.com> | 2019-05-15 15:36:46 -0400 |
commit | bdd1666b3d03d675bdb7f8d92b29f2797acbc5e8 (patch) | |
tree | 0f2206c34aa3c6b80980adaeae170c510689f2b7 /tools/perf | |
parent | 6b89d4c1ae8596a8c9240f169ef108704de373f2 (diff) |
perf annotate: Remove hist__account_cycles() from callback
The hist__account_cycles() function is executed when the
hist_iter__branch_callback() is called.
But it looks it's not necessary. In hist__account_cycles, it already
walks on all branch entries.
This patch moves the hist__account_cycles out of callback, now the data
processing is much faster than before.
Previous code has an issue that the ch[offset].num++ (in
__symbol__account_cycles) is executed repeatedly since
hist__account_cycles is called in each hist_iter__branch_callback, so
the counting of ch[offset].num is not correct (too big).
With this patch, the issue is fixed. And we don't need the code of
"ch->reset >= ch->num / 2" to check if there are too many overlaps (in
annotation__count_and_fill), otherwise some data would be hidden.
Now, we can try, for example:
perf record -b ...
perf annotate or perf report -s symbol
The before/after output should be no change.
v3:
---
Fix the crash in stdio mode.
Like previous code, it needs the checking of ui__has_annotation()
before hist__account_cycles()
v2:
---
1. Cover the similar perf report
2. Remove the checking code "ch->reset >= ch->num / 2"
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1552684577-29041-1-git-send-email-yao.jin@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Diffstat (limited to 'tools/perf')
-rw-r--r-- | tools/perf/builtin-annotate.c | 4 | ||||
-rw-r--r-- | tools/perf/builtin-report.c | 11 | ||||
-rw-r--r-- | tools/perf/util/annotate.c | 2 |
3 files changed, 8 insertions, 9 deletions
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 67f9d9ffacfb..77deb3a40596 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c | |||
@@ -159,8 +159,6 @@ static int hist_iter__branch_callback(struct hist_entry_iter *iter, | |||
159 | struct perf_evsel *evsel = iter->evsel; | 159 | struct perf_evsel *evsel = iter->evsel; |
160 | int err; | 160 | int err; |
161 | 161 | ||
162 | hist__account_cycles(sample->branch_stack, al, sample, false); | ||
163 | |||
164 | bi = he->branch_info; | 162 | bi = he->branch_info; |
165 | err = addr_map_symbol__inc_samples(&bi->from, sample, evsel); | 163 | err = addr_map_symbol__inc_samples(&bi->from, sample, evsel); |
166 | 164 | ||
@@ -199,6 +197,8 @@ static int process_branch_callback(struct perf_evsel *evsel, | |||
199 | if (a.map != NULL) | 197 | if (a.map != NULL) |
200 | a.map->dso->hit = 1; | 198 | a.map->dso->hit = 1; |
201 | 199 | ||
200 | hist__account_cycles(sample->branch_stack, al, sample, false); | ||
201 | |||
202 | ret = hist_entry_iter__add(&iter, &a, PERF_MAX_STACK_DEPTH, ann); | 202 | ret = hist_entry_iter__add(&iter, &a, PERF_MAX_STACK_DEPTH, ann); |
203 | return ret; | 203 | return ret; |
204 | } | 204 | } |
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 4054eb1f98ac..91e27ac297c2 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c | |||
@@ -136,9 +136,6 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter, | |||
136 | if (!ui__has_annotation() && !rep->symbol_ipc) | 136 | if (!ui__has_annotation() && !rep->symbol_ipc) |
137 | return 0; | 137 | return 0; |
138 | 138 | ||
139 | hist__account_cycles(sample->branch_stack, al, sample, | ||
140 | rep->nonany_branch_mode); | ||
141 | |||
142 | if (sort__mode == SORT_MODE__BRANCH) { | 139 | if (sort__mode == SORT_MODE__BRANCH) { |
143 | bi = he->branch_info; | 140 | bi = he->branch_info; |
144 | err = addr_map_symbol__inc_samples(&bi->from, sample, evsel); | 141 | err = addr_map_symbol__inc_samples(&bi->from, sample, evsel); |
@@ -181,9 +178,6 @@ static int hist_iter__branch_callback(struct hist_entry_iter *iter, | |||
181 | if (!ui__has_annotation() && !rep->symbol_ipc) | 178 | if (!ui__has_annotation() && !rep->symbol_ipc) |
182 | return 0; | 179 | return 0; |
183 | 180 | ||
184 | hist__account_cycles(sample->branch_stack, al, sample, | ||
185 | rep->nonany_branch_mode); | ||
186 | |||
187 | bi = he->branch_info; | 181 | bi = he->branch_info; |
188 | err = addr_map_symbol__inc_samples(&bi->from, sample, evsel); | 182 | err = addr_map_symbol__inc_samples(&bi->from, sample, evsel); |
189 | if (err) | 183 | if (err) |
@@ -282,6 +276,11 @@ static int process_sample_event(struct perf_tool *tool, | |||
282 | if (al.map != NULL) | 276 | if (al.map != NULL) |
283 | al.map->dso->hit = 1; | 277 | al.map->dso->hit = 1; |
284 | 278 | ||
279 | if (ui__has_annotation() || rep->symbol_ipc) { | ||
280 | hist__account_cycles(sample->branch_stack, &al, sample, | ||
281 | rep->nonany_branch_mode); | ||
282 | } | ||
283 | |||
285 | ret = hist_entry_iter__add(&iter, &al, rep->max_stack, rep); | 284 | ret = hist_entry_iter__add(&iter, &al, rep->max_stack, rep); |
286 | if (ret < 0) | 285 | if (ret < 0) |
287 | pr_debug("problem adding hist entry, skipping event\n"); | 286 | pr_debug("problem adding hist entry, skipping event\n"); |
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 09762985c713..0b8573fd9b05 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c | |||
@@ -1021,7 +1021,7 @@ static void annotation__count_and_fill(struct annotation *notes, u64 start, u64 | |||
1021 | float ipc = n_insn / ((double)ch->cycles / (double)ch->num); | 1021 | float ipc = n_insn / ((double)ch->cycles / (double)ch->num); |
1022 | 1022 | ||
1023 | /* Hide data when there are too many overlaps. */ | 1023 | /* Hide data when there are too many overlaps. */ |
1024 | if (ch->reset >= 0x7fff || ch->reset >= ch->num / 2) | 1024 | if (ch->reset >= 0x7fff) |
1025 | return; | 1025 | return; |
1026 | 1026 | ||
1027 | for (offset = start; offset <= end; offset++) { | 1027 | for (offset = start; offset <= end; offset++) { |