perf report: fix branch view mode glitches

PATCH In review Stephane Eranian 1 participant 5 messages Mar 12, 2012 – Mar 13, 2012 perf events

Share & tools

Stephane Eranian

author

2012-03-12 15:13 UTC #0 lore raw

cover perf report: fix branch view mode glitches

This small series fixes the following two glitches in perf report while in branch view mode:
- duplicate annotation choice in case the branch source and target are in the same function

- go back two levels when pressing 'q' while annotating

Signed-off-by Stephane Eranian <[email protected]>

Stephane Eranian (2):
perf report: remove duplicate annotate choice in branch view mode perf report: fix annotate double quit issue in branch view mode

 tools/perf/util/ui/browsers/hists.c |   40 ++++++++++++++++++++++++++++------
 1 files changed, 33 insertions(+), 7 deletions(-)

Signature · 1 line

1.7.4.1

Stephane Eranian

author

2012-03-12 15:13 UTC #1 lore raw

PATCH 1/2 perf report: remove duplicate annotate choice in branch view mode

This patch removes the duplicated annotate selection when browsing in branch view mode. If the sym and dso oof the branch source and target are the same, then only one annotate choice is proposed.

Signed-off-by Stephane Eranian <[email protected]>

---
 tools/perf/util/ui/browsers/hists.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

tools/perf/util/ui/browsers/hists.c +2 -0

index 951e2e9..40fd424 100644
@@ -989,6 +989,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			    bi &&
 			    bi->to.sym != NULL &&
 			    !bi->to.map->dso->annotate_warned &&
+			    (bi->to.sym != bi->from.sym ||
+			     bi->to.map->dso != bi->from.map->dso) &&
 				asprintf(&options[nr_options], "Annotate %s",
 					 bi->to.sym->name) > 0)
 				annotate_t = nr_options++;

Signature · 1 line

1.7.4.1

tip-bot for Stephane Eranian

author

2012-03-13 04:50 UTC #2 lore raw

[tip:perf/core] perf report: Remove duplicate annotate choice in branch view mode

Commit-ID:  8bcd65fd29e9abfb633bfd6eedaf24b22e211f3a
Gitweb:     http://git.kernel.org/tip/8bcd65fd29e9abfb633bfd6eedaf24b22e211f3a
Author:     Stephane Eranian <[email protected]>
AuthorDate: Mon, 12 Mar 2012 16:13:29 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 12 Mar 2012 20:46:16 +0100

perf report: Remove duplicate annotate choice in branch view mode

This patch removes the duplicated annotate selection when browsing in branch view mode. If the sym and dso oof the branch source and target are the same, then only one annotate choice is proposed.

Signed-off-by Stephane Eranian <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 tools/perf/util/ui/browsers/hists.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

tools/perf/util/ui/browsers/hists.c +2 -0

index 951e2e9..40fd424 100644
@@ -989,6 +989,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			    bi &&
 			    bi->to.sym != NULL &&
 			    !bi->to.map->dso->annotate_warned &&
+			    (bi->to.sym != bi->from.sym ||
+			     bi->to.map->dso != bi->from.map->dso) &&
 				asprintf(&options[nr_options], "Annotate %s",
 					 bi->to.sym->name) > 0)
 				annotate_t = nr_options++;

Stephane Eranian

author

2012-03-12 15:13 UTC #3 lore raw

PATCH 2/2 perf report: fix annotate double quit issue in branch view mode

In reply to Stephane Eranian

This patch fixes perf report to not go back two levels when pressing the 'q' key while annotating in branch view mode.

When pressing 'q' in annotate mode and if the branch source and target belong to different functions, perf now brings up the annotation popup menu again to offer the option to annotate the other branch source or target.

As part of the code restructuring in perf_evsel__hists_browse() we also fix a memory leak on options[] in case of error.

Signed-off-by Stephane Eranian <[email protected]>

---
 tools/perf/util/ui/browsers/hists.c |   38 ++++++++++++++++++++++++++++------
 1 files changed, 31 insertions(+), 7 deletions(-)

tools/perf/util/ui/browsers/hists.c +31 -7

index 40fd424..de8ece8 100644
@@ -856,6 +856,16 @@ static int hists__browser_title(struct hists *self, char *bf, size_t size,
 	return printed;
 }
 
+static inline void free_popup_options(char **options, int n)
+{
+	int i;
+
+	for (i = 0; i < n; ++i) {
+		free(options[i]);
+		options[i] = NULL;
+	}
+}
+
 static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 				    const char *helpline, const char *ev_name,
 				    bool left_exits,

@@ -866,6 +876,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	struct hist_browser *browser = hist_browser__new(self);
 	struct branch_info *bi;
 	struct pstack *fstack;
+	char *options[16];
+	int nr_options = 0;
 	int key = -1;
 
 	if (browser == NULL)

@@ -877,14 +889,17 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 	ui_helpline__push(helpline);
 
+	memset(options, 0, sizeof(options));
+
 	while (1) {
 		const struct thread *thread = NULL;
 		const struct dso *dso = NULL;
-		char *options[16];
-		int nr_options = 0, choice = 0, i,
+		int choice = 0,
 		    annotate = -2, zoom_dso = -2, zoom_thread = -2,
 		    annotate_f = -2, annotate_t = -2, browse_map = -2;
 
+		nr_options = 0;
+
 		key = hist_browser__run(browser, ev_name, timer, arg, delay_secs);
 
 		if (browser->he_selection != NULL) {

@@ -1023,17 +1038,16 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			browse_map = nr_options++;
 add_exit_option:
 		options[nr_options++] = (char *)"Exit";
-
+retry_popup_menu:
 		choice = ui__popup_menu(nr_options, options);
 
-		for (i = 0; i < nr_options - 1; ++i)
-			free(options[i]);
-
 		if (choice == nr_options - 1)
 			break;
 
-		if (choice == -1)
+		if (choice == -1) {
+			free_popup_options(options, nr_options - 1);
 			continue;
+		}
 
 		if (choice == annotate || choice == annotate_t || choice == annotate_f) {
 			struct hist_entry *he;

@@ -1064,9 +1078,18 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			err = hist_entry__tui_annotate(he, evsel->idx,
 						       timer, arg, delay_secs);
 			he->used = false;
+			/*
+			 * offer option to annotate the other branch source or target
+			 * (if they exists) when returning from annotate
+			 */
+			if ((err == 'q' || err == CTRL('c'))
+			    && annotate_t != -2 && annotate_f != -2)
+				goto retry_popup_menu;
+
 			ui_browser__update_nr_entries(&browser->b, browser->hists->nr_entries);
 			if (err)
 				ui_browser__handle_resize(&browser->b);
+
 		} else if (choice == browse_map)
 			map__browse(browser->selection->map);
 		else if (choice == zoom_dso) {

@@ -1112,6 +1135,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	pstack__delete(fstack);
 out:
 	hist_browser__delete(browser);
+	free_popup_options(options, nr_options - 1);
 	return key;
 }

Signature · 1 line

1.7.4.1

tip-bot for Stephane Eranian

author

2012-03-13 04:51 UTC #4 lore raw

[tip:perf/core] perf report: Fix annotate double quit issue in branch view mode

Commit-ID:  24bff2dc0f77b1f186b7bdf30060caf3df191a68
Gitweb:     http://git.kernel.org/tip/24bff2dc0f77b1f186b7bdf30060caf3df191a68
Author:     Stephane Eranian <[email protected]>
AuthorDate: Mon, 12 Mar 2012 16:13:30 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 12 Mar 2012 20:46:16 +0100

perf report: Fix annotate double quit issue in branch view mode

This patch fixes perf report to not go back two levels when pressing the 'q' key while annotating in branch view mode.

As part of the code restructuring in perf_evsel__hists_browse() we also fix a memory leak on options[] in case of error.

Signed-off-by Stephane Eranian <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 tools/perf/util/ui/browsers/hists.c |   38 ++++++++++++++++++++++++++++------
 1 files changed, 31 insertions(+), 7 deletions(-)

tools/perf/util/ui/browsers/hists.c +31 -7

index 40fd424..de8ece8 100644
@@ -856,6 +856,16 @@ static int hists__browser_title(struct hists *self, char *bf, size_t size,
 	return printed;
 }
 
+static inline void free_popup_options(char **options, int n)
+{
+	int i;
+
+	for (i = 0; i < n; ++i) {
+		free(options[i]);
+		options[i] = NULL;
+	}
+}
+
 static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 				    const char *helpline, const char *ev_name,
 				    bool left_exits,

@@ -866,6 +876,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	struct hist_browser *browser = hist_browser__new(self);
 	struct branch_info *bi;
 	struct pstack *fstack;
+	char *options[16];
+	int nr_options = 0;
 	int key = -1;
 
 	if (browser == NULL)

@@ -877,14 +889,17 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 	ui_helpline__push(helpline);
 
+	memset(options, 0, sizeof(options));
+
 	while (1) {
 		const struct thread *thread = NULL;
 		const struct dso *dso = NULL;
-		char *options[16];
-		int nr_options = 0, choice = 0, i,
+		int choice = 0,
 		    annotate = -2, zoom_dso = -2, zoom_thread = -2,
 		    annotate_f = -2, annotate_t = -2, browse_map = -2;
 
+		nr_options = 0;
+
 		key = hist_browser__run(browser, ev_name, timer, arg, delay_secs);
 
 		if (browser->he_selection != NULL) {

@@ -1023,17 +1038,16 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			browse_map = nr_options++;
 add_exit_option:
 		options[nr_options++] = (char *)"Exit";
-
+retry_popup_menu:
 		choice = ui__popup_menu(nr_options, options);
 
-		for (i = 0; i < nr_options - 1; ++i)
-			free(options[i]);
-
 		if (choice == nr_options - 1)
 			break;
 
-		if (choice == -1)
+		if (choice == -1) {
+			free_popup_options(options, nr_options - 1);
 			continue;
+		}
 
 		if (choice == annotate || choice == annotate_t || choice == annotate_f) {
 			struct hist_entry *he;

@@ -1064,9 +1078,18 @@ do_annotate:
 			err = hist_entry__tui_annotate(he, evsel->idx,
 						       timer, arg, delay_secs);
 			he->used = false;
+			/*
+			 * offer option to annotate the other branch source or target
+			 * (if they exists) when returning from annotate
+			 */
+			if ((err == 'q' || err == CTRL('c'))
+			    && annotate_t != -2 && annotate_f != -2)
+				goto retry_popup_menu;
+
 			ui_browser__update_nr_entries(&browser->b, browser->hists->nr_entries);
 			if (err)
 				ui_browser__handle_resize(&browser->b);
+
 		} else if (choice == browse_map)
 			map__browse(browser->selection->map);
 		else if (choice == zoom_dso) {

@@ -1112,6 +1135,7 @@ out_free_stack:
 	pstack__delete(fstack);
 out:
 	hist_browser__delete(browser);
+	free_popup_options(options, nr_options - 1);
 	return key;
 }