mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/3] perf tools: Fix double count of total period
@ 2011-10-15 18:36 Frederic Weisbecker
  2011-10-15 18:36 ` [PATCH 2/3] perf tools: Reset hists number of entries before collapsing Frederic Weisbecker
  2011-10-15 18:36 ` [PATCH 3/3] perf tools: Fix leaked account of hist period on collapsing Frederic Weisbecker
  0 siblings, 2 replies; 3+ messages in thread
From: Frederic Weisbecker @ 2011-10-15 18:36 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
	David Ahern, Stephane Eranian

When we resort the entries to fix the order of the hists after
we collapsed them, we count all the hists one more time,
recalculing the cols len, the stats, etc...

However we forget to reset the total period before doing that.
So the ending count is buggy.

When we resort the entries, we don't change their total numbers
or their content. So we can avoid to recompute the total period
and the len of the cols there.

This fixes the issue.

Before:

	# Events: 23  cycles
	#
	# Overhead  Command      Shared Object                 Symbol
	# ........  .......  .................  .....................
	#
	    18.35%     perf  [kernel.kallsyms]  [k] add_preempt_count
	    15.76%     perf  [kernel.kallsyms]  [k] lock_is_held
	    15.22%     sshd  [kernel.kallsyms]  [k] register_lock_class
	     0.17%  swapper  [kernel.kallsyms]  [k] lock_release
	     0.17%     perf  [kernel.kallsyms]  [k] lock_release
	     0.17%  swapper  [kernel.kallsyms]  [k] __perf_event_enable
	     0.16%  swapper  [kernel.kallsyms]  [k] native_write_msr_safe
	     0.00%     perf  [kernel.kallsyms]  [k] native_write_msr_safe

After:

	# Events: 23  cycles
	#
	# Overhead  Command      Shared Object                 Symbol
	# ........  .......  .................  .....................
	#
	    36.70%     perf  [kernel.kallsyms]  [k] add_preempt_count
	    31.52%     perf  [kernel.kallsyms]  [k] lock_is_held
	    30.43%     sshd  [kernel.kallsyms]  [k] register_lock_class
	     0.35%  swapper  [kernel.kallsyms]  [k] lock_release
	     0.34%     perf  [kernel.kallsyms]  [k] lock_release
	     0.34%  swapper  [kernel.kallsyms]  [k] __perf_event_enable
	     0.32%  swapper  [kernel.kallsyms]  [k] native_write_msr_safe
	     0.01%     perf  [kernel.kallsyms]  [k] native_write_msr_safe

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: David Ahern <dsahern@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/hist.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index a7193c5..bac6520 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -413,15 +413,11 @@ static void __hists__output_resort(struct hists *hists, bool threaded)
 	next = rb_first(root);
 	hists->entries = RB_ROOT;
 
-	hists->nr_entries = 0;
-	hists__reset_col_len(hists);
-
 	while (next) {
 		n = rb_entry(next, struct hist_entry, rb_node_in);
 		next = rb_next(&n->rb_node_in);
 
 		__hists__insert_output_entry(&hists->entries, n, min_callchain_hits);
-		hists__inc_nr_entries(hists, n);
 	}
 }
 
-- 
1.7.5.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/3] perf tools: Reset hists number of entries before collapsing
  2011-10-15 18:36 [PATCH 1/3] perf tools: Fix double count of total period Frederic Weisbecker
@ 2011-10-15 18:36 ` Frederic Weisbecker
  2011-10-15 18:36 ` [PATCH 3/3] perf tools: Fix leaked account of hist period on collapsing Frederic Weisbecker
  1 sibling, 0 replies; 3+ messages in thread
From: Frederic Weisbecker @ 2011-10-15 18:36 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
	David Ahern, Stephane Eranian

When we collapse entries, we re-compute their total numbers
and period but we forget to reset the total number of entries
before.

Fix this so that the browser sees the correct number of entries.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: David Ahern <dsahern@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/hist.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index bac6520..cef40d8 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -344,6 +344,7 @@ static void __hists__collapse_resort(struct hists *hists, bool threaded)
 
 	root = hists__get_rotate_entries_in(hists);
 	next = rb_first(root);
+	hists->nr_entries = 0;
 	hists->stats.total_period = 0;
 
 	while (next) {
-- 
1.7.5.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 3/3] perf tools: Fix leaked account of hist period on collapsing
  2011-10-15 18:36 [PATCH 1/3] perf tools: Fix double count of total period Frederic Weisbecker
  2011-10-15 18:36 ` [PATCH 2/3] perf tools: Reset hists number of entries before collapsing Frederic Weisbecker
@ 2011-10-15 18:36 ` Frederic Weisbecker
  1 sibling, 0 replies; 3+ messages in thread
From: Frederic Weisbecker @ 2011-10-15 18:36 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
	David Ahern, Stephane Eranian

When we collapse the hists, we forget to account the part that
is collapsed into the other. This messes up the stats everytime
we sort by at least by comm.

Fix this by acccounting the collapsed side.

Before:

	perf report -s comm --stdio

	# Events: 26K cycles
	#
	# Overhead  Command
	# ........  .......
	#
	   46388.16%     perf

After:
	perf report -s comm --stdio

	# Events: 26K cycles
	#
	# Overhead  Command
	# ........  .......
	#
	   100.00%     perf

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: David Ahern <dsahern@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/hist.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index cef40d8..57d6e78 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -297,6 +297,8 @@ static bool hists__collapse_insert_entry(struct hists *hists,
 
 		if (!cmp) {
 			iter->period += he->period;
+			if (!iter->filtered)
+				hists->stats.total_period += he->period;
 			iter->nr_events += he->nr_events;
 			if (symbol_conf.use_callchain) {
 				callchain_cursor_reset(&hists->callchain_cursor);
-- 
1.7.5.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-10-15 18:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-15 18:36 [PATCH 1/3] perf tools: Fix double count of total period Frederic Weisbecker
2011-10-15 18:36 ` [PATCH 2/3] perf tools: Reset hists number of entries before collapsing Frederic Weisbecker
2011-10-15 18:36 ` [PATCH 3/3] perf tools: Fix leaked account of hist period on collapsing Frederic Weisbecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®