diff mbox series

perf report: Fix hierarchy mode on pipe input

Message ID 20231025003121.2811738-1-namhyung@kernel.org
State New
Headers
Series perf report: Fix hierarchy mode on pipe input |

Commit Message

Namhyung Kim Oct. 25, 2023, 12:31 a.m. UTC 
  The hierarchy mode needs to setup output formats for each evsel.
Normally setup_sorting() handles this at the beginning, but it cannot
do that if data comes from a pipe since there's no evsel info before
reading the data.  And then perf report cannot process the samples
in hierarchy mode and think as if there's no sample.

Let's check the condition and setup the output formats after reading
data so that it can find evsels.

Before:

  $ ./perf record -o- true | ./perf report -i- --hierarchy -q
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  Error:
  The - data has no samples!

After:

  $ ./perf record -o- true | ./perf report -i- --hierarchy -q
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
      94.76%        true
         94.76%        [kernel.kallsyms]
            94.76%        [k] filemap_fault
       5.24%        perf-ex
          5.24%        [kernel.kallsyms]
             5.06%        [k] __memset
             0.18%        [k] native_write_msr

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Ian Rogers Oct. 25, 2023, 3:42 a.m. UTC | #1 
On Tue, Oct 24, 2023 at 5:31 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> The hierarchy mode needs to setup output formats for each evsel.
> Normally setup_sorting() handles this at the beginning, but it cannot
> do that if data comes from a pipe since there's no evsel info before
> reading the data.  And then perf report cannot process the samples
> in hierarchy mode and think as if there's no sample.
>
> Let's check the condition and setup the output formats after reading
> data so that it can find evsels.
>
> Before:
>
>   $ ./perf record -o- true | ./perf report -i- --hierarchy -q
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.000 MB - ]
>   Error:
>   The - data has no samples!
>
> After:
>
>   $ ./perf record -o- true | ./perf report -i- --hierarchy -q
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.000 MB - ]
>       94.76%        true
>          94.76%        [kernel.kallsyms]
>             94.76%        [k] filemap_fault
>        5.24%        perf-ex
>           5.24%        [kernel.kallsyms]
>              5.06%        [k] __memset
>              0.18%        [k] native_write_msr
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-report.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index dcedfe00f04d..ca8f2331795c 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -691,10 +691,25 @@ static int report__browse_hists(struct report *rep)
>
>  static int report__collapse_hists(struct report *rep)
>  {
> +       struct perf_session *session = rep->session;
> +       struct evlist *evlist = session->evlist;
>         struct ui_progress prog;
>         struct evsel *pos;
>         int ret = 0;
>
> +       /*
> +        * The pipe data needs to setup hierarchy hpp formats now, because it
> +        * cannot know about evsels in the data before reading the data.  The
> +        * normal file data saves the event (attribute) info in the header
> +        * section, but pipe does not have the luxury.
> +        */
> +       if (perf_data__is_pipe(session->data)) {
> +               if (perf_hpp__setup_hists_formats(&perf_hpp_list, evlist) < 0) {
> +                       ui__error("Failed to setup hierachy output formats\n");
> +                       return -1;
> +               }
> +       }
> +
>         ui_progress__init(&prog, rep->nr_entries, "Merging related events...");
>
>         evlist__for_each_entry(rep->session->evlist, pos) {
> --
> 2.42.0.758.gaed0368e0e-goog
>
Namhyung Kim Oct. 26, 2023, 5:11 p.m. UTC | #2 
On Tue, 24 Oct 2023 17:31:21 -0700, Namhyung Kim wrote:
> The hierarchy mode needs to setup output formats for each evsel.
> Normally setup_sorting() handles this at the beginning, but it cannot
> do that if data comes from a pipe since there's no evsel info before
> reading the data.  And then perf report cannot process the samples
> in hierarchy mode and think as if there's no sample.
> 
> Let's check the condition and setup the output formats after reading
> data so that it can find evsels.
> 
> [...]

Applied to perf-tools-next, thanks!
diff mbox series

Patch

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index dcedfe00f04d..ca8f2331795c 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -691,10 +691,25 @@  static int report__browse_hists(struct report *rep)
 
 static int report__collapse_hists(struct report *rep)
 {
+	struct perf_session *session = rep->session;
+	struct evlist *evlist = session->evlist;
 	struct ui_progress prog;
 	struct evsel *pos;
 	int ret = 0;
 
+	/*
+	 * The pipe data needs to setup hierarchy hpp formats now, because it
+	 * cannot know about evsels in the data before reading the data.  The
+	 * normal file data saves the event (attribute) info in the header
+	 * section, but pipe does not have the luxury.
+	 */
+	if (perf_data__is_pipe(session->data)) {
+		if (perf_hpp__setup_hists_formats(&perf_hpp_list, evlist) < 0) {
+			ui__error("Failed to setup hierachy output formats\n");
+			return -1;
+		}
+	}
+
 	ui_progress__init(&prog, rep->nr_entries, "Merging related events...");
 
 	evlist__for_each_entry(rep->session->evlist, pos) {