All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Ingo Molnar <mingo@kernel.org>, Paul Mackerras <paulus@samba.org>,
	Namhyung Kim <namhyung.kim@lge.com>,
	LKML <linux-kernel@vger.kernel.org>,
	David Ahern <dsahern@gmail.com>, Andi Kleen <andi@firstfloor.org>
Subject: Re: [PATCH 3/9] perf tools: Account entry stats when it's added to the output tree
Date: Tue, 22 Apr 2014 16:54:49 +0200	[thread overview]
Message-ID: <20140422145449.GI1104@krava.brq.redhat.com> (raw)
In-Reply-To: <1398156591-11001-4-git-send-email-namhyung@kernel.org>

On Tue, Apr 22, 2014 at 05:49:45PM +0900, Namhyung Kim wrote:

SNIP

>  }
>  
>  static int process_sample_event(struct perf_tool *tool,
> @@ -234,19 +230,17 @@ static int __cmd_annotate(struct perf_annotate *ann)
>  	total_nr_samples = 0;
>  	evlist__for_each(session->evlist, pos) {
>  		struct hists *hists = &pos->hists;
> -		u32 nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
>  
> -		if (nr_samples > 0) {

so this condition of having some data is handled in the
resort I guess.. hm, it will just iterate 0 times ;-)

> -			total_nr_samples += nr_samples;
> -			hists__collapse_resort(hists, NULL);
> -			hists__output_resort(hists);
> +		hists__collapse_resort(hists, NULL);
> +		hists__output_resort(hists);
>  
> -			if (symbol_conf.event_group &&
> -			    !perf_evsel__is_group_leader(pos))
> -				continue;
> +		if (symbol_conf.event_group &&
> +		    !perf_evsel__is_group_leader(pos))
> +			continue;
>  
> -			hists__find_annotations(hists, pos, ann);
> -		}
> +		hists__find_annotations(hists, pos, ann);
> +
> +		total_nr_samples += hists->stats.nr_events[PERF_RECORD_SAMPLE];
>  	}
>  
>  	if (total_nr_samples == 0) {
>  

SNIP

> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 1c77714f668d..f955ae5a41c5 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -344,9 +344,11 @@ void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h)

I was wondering for a while about the name of this function
since it no longer increments only nr_entries.. but could not
think about any good replacement ;-)

We could also add hists__reset_nr_entries for the zeroing code.


>  		hists__calc_col_len(hists, h);

also having hists__calc_col_len called here seems strange

thanks,
jirka

  reply	other threads:[~2014-04-22 14:55 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-22  8:49 [PATCHSET 0/9] perf tools: Fixup for the --percentage change Namhyung Kim
2014-04-22  8:49 ` [PATCH 1/9] perf report: Count number of entries and samples separately Namhyung Kim
2014-04-22 14:51   ` Jiri Olsa
2014-04-23  4:52     ` Namhyung Kim
2014-04-22 16:43   ` Jiri Olsa
2014-04-22  8:49 ` [PATCH 2/9] perf hists: Introduce hists__add_nr_events() Namhyung Kim
2014-04-22 14:52   ` Jiri Olsa
2014-04-23  4:53     ` Namhyung Kim
2014-04-22  8:49 ` [PATCH 3/9] perf tools: Account entry stats when it's added to the output tree Namhyung Kim
2014-04-22 14:54   ` Jiri Olsa [this message]
2014-04-23  4:58     ` Namhyung Kim
2014-04-22 17:10   ` Jiri Olsa
2014-04-23  5:14     ` Namhyung Kim
2014-04-22  8:49 ` [PATCH 4/9] perf tools: Introduce hists__inc_dump_events() Namhyung Kim
2014-04-22 16:53   ` Jiri Olsa
2014-04-23  5:58     ` Namhyung Kim
2014-04-22  8:49 ` [PATCH 5/9] perf hists: Add missing update on nr_non_filtered_entries Namhyung Kim
2014-04-22  8:49 ` [PATCH 6/9] perf ui/tui: Fix off-by-one in hist_browser__update_nr_entries() Namhyung Kim
2014-04-22  8:49 ` [PATCH 7/9] perf ui/tui: Rename hist_browser__update_nr_entries() Namhyung Kim
2014-04-22  8:49 ` [PATCH 8/9] perf top/tui: Update nr_entries properly after a filter is applied Namhyung Kim
2014-04-22  8:49 ` [PATCH 9/9] perf hists/tui: Count callchain rows separately Namhyung Kim
2014-04-22 17:39   ` Jiri Olsa
2014-04-22  9:55 ` [PATCHSET 0/9] perf tools: Fixup for the --percentage change Ingo Molnar
2014-04-23  4:49   ` Namhyung Kim
2014-04-23  6:09     ` Ingo Molnar
2014-04-25  7:53       ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140422145449.GI1104@krava.brq.redhat.com \
    --to=jolsa@redhat.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=andi@firstfloor.org \
    --cc=dsahern@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung.kim@lge.com \
    --cc=namhyung@kernel.org \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.