Skip to contentSkip to conversation

perf report: Make --branch-history work without callgraphs(-g) option in perf record

PATCH Superseded Jin Yao 2 participants 3 messages May 4, 2017 – May 5, 2017 perf events

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jin Yao

author
#0 lore raw

PATCH 1/1 perf report: Make --branch-history work without callgraphs(-g) option in perf record

perf record -b -g <command>
perf report --branch-history

This merges the LBRs with the callgraphs.

However it would be nice if it also works without callgraphs (-g) set in perf record, so that only the LBRs are displayed. But currently perf report errors in this case. For example,

perf record -b <command>
perf report --branch-history

Error:
Selected -g or --branch-history but no callchain data. Did you call 'perf record' without -g?

This patch displays the LBRs only even if callgraphs(-g) is not enabled in perf record.

---
 tools/perf/builtin-report.c |  6 ++++--
 tools/perf/util/callchain.c |  7 ++++---
 tools/perf/util/hist.c      |  2 ++
 tools/perf/util/machine.c   | 13 ++++++++++++-
 4 files changed, 22 insertions(+), 6 deletions(-)
tools/perf/builtin-report.c +4 -2
index 22478ff..7f82369 100644
@@ -259,7 +259,8 @@ static int report__setup_sample_type(struct report *rep)
 				    "'perf record' without -g?\n");
 			return -EINVAL;
 		}
-		if (symbol_conf.use_callchain) {
+		if (symbol_conf.use_callchain &&
+			!symbol_conf.show_branchflag_count) {
 			ui__error("Selected -g or --branch-history but no "
 				  "callchain data. Did\n"
 				  "you call 'perf record' without -g?\n");
@@ -397,7 +398,8 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
 
 		hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
 		hists__fprintf(hists, !quiet, 0, 0, rep->min_percent, stdout,
-			       symbol_conf.use_callchain);
+			       symbol_conf.use_callchain |
+			       symbol_conf.show_branchflag_count);
 		fprintf(stdout, "\n\n");
 	}
 
tools/perf/util/callchain.c +4 -3
index 81fc29a..08d3abf 100644
@@ -993,11 +993,11 @@ int sample__resolve_callchain(struct perf_sample *sample,
 			      struct perf_evsel *evsel, struct addr_location *al,
 			      int max_stack)
 {
-	if (sample->callchain == NULL)
+	if (sample->callchain == NULL && !symbol_conf.show_branchflag_count)
 		return 0;
 
 	if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain ||
-	    perf_hpp_list.parent) {
+	    perf_hpp_list.parent || symbol_conf.show_branchflag_count) {
 		return thread__resolve_callchain(al->thread, cursor, evsel, sample,
 						 parent, al, max_stack);
 	}
@@ -1006,7 +1006,8 @@ int sample__resolve_callchain(struct perf_sample *sample,
 
 int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *sample)
 {
-	if (!symbol_conf.use_callchain || sample->callchain == NULL)
+	if ((!symbol_conf.use_callchain || sample->callchain == NULL) &&
+		!symbol_conf.show_branchflag_count)
 		return 0;
 	return callchain_append(he->callchain, &callchain_cursor, sample->period);
 }
tools/perf/util/hist.c +2 -0
index cf0186a..8b045a5 100644
@@ -1762,6 +1762,8 @@ void perf_evsel__output_resort(struct perf_evsel *evsel, struct ui_progress *pro
 	else
 		use_callchain = symbol_conf.use_callchain;
 
+	use_callchain |= symbol_conf.show_branchflag_count;
+
 	output_resort(evsel__hists(evsel), prog, use_callchain, NULL);
 }
 
tools/perf/util/machine.c +12 -1
index a98f55a..792ddcd 100644
@@ -1901,13 +1901,16 @@ static int thread__resolve_callchain_sample(struct thread *thread,
 {
 	struct branch_stack *branch = sample->branch_stack;
 	struct ip_callchain *chain = sample->callchain;
-	int chain_nr = chain->nr;
+	int chain_nr = 0;
 	u8 cpumode = PERF_RECORD_MISC_USER;
 	int i, j, err, nr_entries;
 	int skip_idx = -1;
 	int first_call = 0;
 	int nr_loop_iter;
 
+	if (chain)
+		chain_nr = chain->nr;
+
 	if (perf_evsel__has_branch_callstack(evsel)) {
 		err = resolve_lbr_callchain_sample(thread, cursor, sample, parent,
 						   root_al, max_stack);
@@ -1945,6 +1948,10 @@ static int thread__resolve_callchain_sample(struct thread *thread,
 		for (i = 0; i < nr; i++) {
 			if (callchain_param.order == ORDER_CALLEE) {
 				be[i] = branch->entries[i];
+
+				if (chain == NULL)
+					continue;
+
 				/*
 				 * Check for overlap into the callchain.
 				 * The return address is one off compared to
@@ -1998,6 +2005,10 @@ static int thread__resolve_callchain_sample(struct thread *thread,
 			if (err)
 				return err;
 		}
+
+		if (chain_nr == 0)
+			return 0;
+
 		chain_nr -= nr;
 	}
 
Signature · 1 line
2.7.4
#1 lore raw
Quoted 32 lines · CEST Jin Yao

perf record -b -g <command>
perf report --branch-history

This merges the LBRs with the callgraphs.

However it would be nice if it also works without callgraphs (-g) set in perf record, so that only the LBRs are displayed. But currently perf report errors in this case. For example,

perf record -b <command>
perf report --branch-history

Error:
Selected -g or --branch-history but no callchain data. Did you call 'perf record' without -g?

This patch displays the LBRs only even if callgraphs(-g) is not enabled in perf record.

---
 tools/perf/builtin-report.c |  6 ++++--
 tools/perf/util/callchain.c |  7 ++++---
 tools/perf/util/hist.c      |  2 ++
 tools/perf/util/machine.c   | 13 ++++++++++++-
 4 files changed, 22 insertions(+), 6 deletions(-)
tools/perf/builtin-report.c +0 -0
index 22478ff..7f82369 100644

tools/perf/builtin-report.c static int report__setup_sample_type(struct report *rep) · line 266

Quoted 9 lines
tools/perf/builtin-report.c +2 -1
@@ -259,7 +259,8 @@ static int report__setup_sample_type(struct report *rep)
 "'perf record' without -g?\n");
 			return -EINVAL;
 		}
-		if (symbol_conf.use_callchain) {
+		if (symbol_conf.use_callchain &&
+			!symbol_conf.show_branchflag_count) {
 			ui__error("Selected -g or --branch-history but no "
 				  "callchain data. Did\n"
 				  "you call 'perf record' without -g?\n");

The --branch-history part of this error message is now stale, no?

tools/perf/builtin-report.c static int perf_evlist__tty_browse_hists(struct · line 405

Quoted 10 lines
tools/perf/builtin-report.c +2 -1
@@ -397,7 +398,8 @@ static int perf_evlist__tty_browse_hists(struct
 perf_evlist *evlist,
 
 		hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
 		hists__fprintf(hists, !quiet, 0, 0, rep->min_percent, stdout,
-			       symbol_conf.use_callchain);
+			       symbol_conf.use_callchain |
+			       symbol_conf.show_branchflag_count);
 		fprintf(stdout, "\n\n");
 	}

Not sure about the coding style here, but shouldn't this be a boolean or operator here '||' - semantically? Functionality wise it will be the same, I guess?

Quoted 85 lines
tools/perf/util/callchain.c +4 -3
index 81fc29a..08d3abf 100644
@@ -993,11 +993,11 @@ int sample__resolve_callchain(struct perf_sample
 *sample, struct perf_evsel *evsel, struct addr_location *al,
 			      int max_stack)
 {
-	if (sample->callchain == NULL)
+	if (sample->callchain == NULL && !symbol_conf.show_branchflag_count)
 		return 0;
 
 	if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain ||
-	    perf_hpp_list.parent) {
+	    perf_hpp_list.parent || symbol_conf.show_branchflag_count) {
 		return thread__resolve_callchain(al->thread, cursor, evsel, sample,
 						 parent, al, max_stack);
 	}
@@ -1006,7 +1006,8 @@ int sample__resolve_callchain(struct perf_sample
 *sample,
 
 int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample
 *sample) {
-	if (!symbol_conf.use_callchain || sample->callchain == NULL)
+	if ((!symbol_conf.use_callchain || sample->callchain == NULL) &&
+		!symbol_conf.show_branchflag_count)
 		return 0;
 	return callchain_append(he->callchain, &callchain_cursor, sample->period);

}

tools/perf/util/hist.c +2 -0
index cf0186a..8b045a5 100644
@@ -1762,6 +1762,8 @@ void perf_evsel__output_resort(struct perf_evsel
 *evsel, struct ui_progress *pro else
 		use_callchain = symbol_conf.use_callchain;
 
+	use_callchain |= symbol_conf.show_branchflag_count;
+
 	output_resort(evsel__hists(evsel), prog, use_callchain, NULL);
 }
 
tools/perf/util/machine.c +12 -1
index a98f55a..792ddcd 100644
@@ -1901,13 +1901,16 @@ static int thread__resolve_callchain_sample(struct
 thread *thread, {
 	struct branch_stack *branch = sample->branch_stack;
 	struct ip_callchain *chain = sample->callchain;
-	int chain_nr = chain->nr;
+	int chain_nr = 0;
 	u8 cpumode = PERF_RECORD_MISC_USER;
 	int i, j, err, nr_entries;
 	int skip_idx = -1;
 	int first_call = 0;
 	int nr_loop_iter;
 
+	if (chain)
+		chain_nr = chain->nr;
+
 	if (perf_evsel__has_branch_callstack(evsel)) {
 		err = resolve_lbr_callchain_sample(thread, cursor, sample, parent,
 						   root_al, max_stack);
@@ -1945,6 +1948,10 @@ static int thread__resolve_callchain_sample(struct
 thread *thread, for (i = 0; i < nr; i++) {
 			if (callchain_param.order == ORDER_CALLEE) {
 				be[i] = branch->entries[i];
+
+				if (chain == NULL)
+					continue;
+
 				/*
 				 * Check for overlap into the callchain.
 				 * The return address is one off compared to
@@ -1998,6 +2005,10 @@ static int thread__resolve_callchain_sample(struct
 thread *thread, if (err)
 				return err;
 		}
+
+		if (chain_nr == 0)
+			return 0;
+
 		chain_nr -= nr;
 	}
Signature · 4 lines
Milian Wolff | [email protected] | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
author
#2 lore raw

SNIP

Quoted 7 lines

tools/perf/builtin-report.c static int report__setup_sample_type(struct report *rep) · line 266

Quoted 6 lines
tools/perf/builtin-report.c +2 -1
-		if (symbol_conf.use_callchain) {
+		if (symbol_conf.use_callchain &&
+			!symbol_conf.show_branchflag_count) {
  			ui__error("Selected -g or --branch-history but no "
  				  "callchain data. Did\n"
  				  "you call 'perf record' without -g?\n");

The --branch-history part of this error message is now stale, no?

Yes, the error message is obsolete. I will change it in next version.

Quoted 4 lines
Quoted 4 lines
(quoted diff) +0 -0
@@ -397,7 +398,8 @@ static int perf_evlist__tty_browse_hists(struct
 perf_evlist *evlist,
 
Quoted 10 lines

tools/perf/builtin-report.c static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist, · line 404

Quoted 7 lines
tools/perf/builtin-report.c +2 -1
  		hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
  		hists__fprintf(hists, !quiet, 0, 0, rep->min_percent, stdout,
-			       symbol_conf.use_callchain);
+			       symbol_conf.use_callchain |
+			       symbol_conf.show_branchflag_count);
  		fprintf(stdout, "\n\n");
  	}

Not sure about the coding style here, but shouldn't this be a boolean or operator here '||' - semantically? Functionality wise it will be the same, I guess?

Should be same. While I will follow your comments to change it to '||'.

I will wait other comments for some days and then send out the next version.

Thanks
Jin Yao