[PATCH] Fix behavior and documentation for '--threshold' option

Fix behavior and documentation for '--threshold' option

A user reported some issues with how the opreport and opannotate
'--threshold' option was working. He was using operf to collect
a profile using mulitiple events (PM_CMPLU_STALL_REJECT_LHS and
PM_MRK_ST_FWD). For a particular symbol in his profile data, he
had 0% for PM_CMPLU_STALL_REJECT_LHS and 12% for PM_MRK_ST_FWD.
But when he ran 'opannotate --assembly -t 1', he was surprised to
see that the function in question was not in the output at all,
even though the ratio of samples for the PM_CMPLU_STALL_REJECT_LHS
was well above the 1% threshold.

The events are stored in alphabetical order in a C++ set. When
applying the threshold level against a symbol, the code was only
looking at the ratio of samples for the first event in the set
(PM_CMPLU_STALL_REJECT_LHS, in this case). This is not the intended
behavior (IMHO), so this patch looks at all ratios for every event
and will only filter out the sample data for a given symbol if none
of the events meets the threshold.

This issue applies to opreport as well, and the same fix works for
both opreport and 'opannotate --assembly'.

On the other hand, 'opannotate --source' applies the threshold to
a given source file (contrary to the man page). The same problem
exists there, where annotation for a given source file was not displayed
if the ratio of samples for the first event in the set did not meet the
specified threshold. This patch fixes that problem as well.

This patch also updates the man pages for opreport and opannotate, as
well as the oprofile user manual, to better explain how the theshold
option works.

Signed-off-by: Maynard Johnson <[email protected]>
---
 doc/opannotate.1.in         |    9 +++++++--
 doc/opreport.1.in           |    3 ++-
 doc/oprofile.xml            |   13 ++++++++++---
 libpp/profile_container.cpp |   23 +++++++++++++++--------
 4 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/doc/opannotate.1.in b/doc/opannotate.1.in
index e0ae1cd..98eda51 100644
--- a/doc/opannotate.1.in
+++ b/doc/opannotate.1.in
@@ -139,8 +139,13 @@ for the binaries.
 .br
 .TP
 .BI "--threshold / -t [percentage]"
-Only output data for symbols that have more than the given percentage
-of total samples.
+For annotated assembly, only output data for symbols that have more than the given percentage
+of total samples. For profiles using multiple events, if the threshold is reached
+for any event, then all sample data for the symbol is shown.
+
+For annotated source, only output data for source files that have more than the given percentage
+of total samples. For profiles using multiple events, if the threshold is reached
+for any event, then all sample data for the source file is shown.
 .br
 .TP
 .BI "--verbose / -V [options]"
diff --git a/doc/opreport.1.in b/doc/opreport.1.in
index 39374c4..0627aa9 100644
--- a/doc/opreport.1.in
+++ b/doc/opreport.1.in
@@ -130,7 +130,8 @@ This difference is typically very small and can be ignored.
 .TP
 .BI "--threshold / -t [percentage]"
 Only output data for symbols that have more than the given percentage
-of total samples.
+of total samples. For profiles using multiple events, if the threshold is reached
+for any event, then all sample data for the symbol is shown.
 .br
 .TP
 .BI "--verbose / -V [options]"
diff --git a/doc/oprofile.xml b/doc/oprofile.xml
index 435ad36..01cd309 100644
--- a/doc/oprofile.xml
+++ b/doc/oprofile.xml
@@ -1680,7 +1680,8 @@ List per-symbol information instead of a binary image summary.
 </para></listitem></varlistentry>
 <varlistentry><term><option>--threshold / -t [percentage]</option></term><listitem><para>
 Only output data for symbols that have more than the given percentage
-of total samples.
+of total samples. For profiles using multiple events, if the threshold is reached
+for any event, then all sample data for the symbol is shown.
 </para></listitem></varlistentry>
 <varlistentry><term><option>--verbose / -V [options]</option></term><listitem><para>
 Give verbose debugging output.
@@ -1886,8 +1887,14 @@ first. If that directory does not exist, the standard session-dir of
 as the session directory.
 </para></listitem></varlistentry>
 <varlistentry><term><option>--threshold / -t [percentage]</option></term><listitem><para>
-Only output data for symbols that have more than the given percentage
-of total samples.
+For annotated assembly, only output data for symbols that have more than the given percentage
+of total samples. For profiles using multiple events, if the threshold is reached
+for any event, then all sample data for the symbol is shown.
+</para>
+<para>
+For annotated source, only output data for source files that have more than the given percentage
+of total samples. For profiles using multiple events, if the threshold is reached
+for any event, then all sample data for the source file is shown.
 </para></listitem></varlistentry>
 <varlistentry><term><option>--verbose / -V [options]</option></term><listitem><para>
 Give verbose debugging output.
diff --git a/libpp/profile_container.cpp b/libpp/profile_container.cpp
index 88de266..dca6fdd 100644
--- a/libpp/profile_container.cpp
+++ b/libpp/profile_container.cpp
@@ -183,13 +183,16 @@ profile_container::select_symbols(symbol_choice & choice) const
 		    && (image_names.name(it->image_name) != choice.image_name))
 			continue;
 
-		double const percent =
-			op_ratio(it->sample.counts[0], total_count[0]);
+		for (size_t j = 0; j < total_count.size(); j++) {
+			double const percent =
+					op_ratio(it->sample.counts[j], total_count[j]);
 
-		if (percent >= threshold) {
-			result.push_back(&*it);
+			if (percent >= threshold) {
+				result.push_back(&*it);
 
-			choice.hints = it->output_hint(choice.hints);
+				choice.hints = it->output_hint(choice.hints);
+				break;
+			}
 		}
 	}
 
@@ -226,9 +229,13 @@ profile_container::select_filename(double threshold) const
 		// FIXME: is samples_count() the right interface now ?
 		count_array_t counts = samples_count(*it);
 
-		double const ratio = op_ratio(counts[0], total_count[0]);
-		filename_by_samples const f(*it, ratio);
-
+		double highest_ratio = 0.0;
+		for (size_t j = 0; j < total_count.size(); j++ ) {
+			double const ratio = op_ratio(counts[j], total_count[j]);
+			if (ratio > highest_ratio)
+				highest_ratio = ratio;
+		}
+		filename_by_samples const f(*it, highest_ratio);
 		file_by_samples.push_back(f);
 	}
 
-- 
1.7.1


------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/