linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>, David Ahern <dsahern@gmail.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Stephane Eranian <eranian@google.com>,
	kernel-team@lge.com
Subject: Re: [PATCH 14/20] perf annotate: Pass struct annotation_options to map_symbol__annotation_dump
Date: Mon, 6 Aug 2018 12:22:19 -0300	[thread overview]
Message-ID: <20180806152219.GA14525@kernel.org> (raw)
In-Reply-To: <20180806142447.GB16446@krava>

Em Mon, Aug 06, 2018 at 04:24:47PM +0200, Jiri Olsa escreveu:
> On Mon, Aug 06, 2018 at 10:45:07PM +0900, Namhyung Kim wrote:
> > Hi Jiri,
> > 
> > On Sat, Aug 04, 2018 at 03:05:15PM +0200, Jiri Olsa wrote:
> > > Passing struct annotation_options to map_symbol__annotation_dump,
> > > to carry on and pass the percent_type value.
> > > 
> > > Link: http://lkml.kernel.org/n/tip-5toohgdkgpk3vn6zebusr3bb@git.kernel.org
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > 
> > [SNIP]
> > > @@ -2523,7 +2526,7 @@ static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
> > >  
> > >  static void __annotation_line__write(struct annotation_line *al, struct annotation *notes,
> > >  				     bool first_line, bool current_entry, bool change_color, int width,
> > > -				     void *obj,
> > > +				     void *obj, unsigned int percent_type,
> > >  				     int  (*obj__set_color)(void *obj, int color),
> > >  				     void (*obj__set_percent_color)(void *obj, double percent, bool current),
> > >  				     int  (*obj__set_jumps_percent_color)(void *obj, int nr, bool current),
> > > @@ -2531,7 +2534,7 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
> > >  				     void (*obj__write_graph)(void *obj, int graph))
> > >  
> > >  {
> > > -	double percent_max = annotation_line__max_percent(al, notes);
> > > +	double percent_max = annotation_line__max_percent(al, notes, percent_type);
> > >  	int pcnt_width = annotation__pcnt_width(notes),
> > >  	    cycles_width = annotation__cycles_width(notes);
> > >  	bool show_title = false;
> > > @@ -2552,8 +2555,7 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
> > >  		for (i = 0; i < notes->nr_events; i++) {
> > >  			double percent;
> > >  
> > > -			percent = annotation_data__percent(&al->data[i],
> > > -							   PERCENT_HITS_LOCAL);
> > > +			percent = annotation_data__percent(&al->data[i], percent_type);
> > >  
> > >  			obj__set_percent_color(obj, percent, current_entry);
> > >  			if (notes->options->show_total_period) {
> > > @@ -2680,13 +2682,15 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
> > >  }
> > >  
> > >  void annotation_line__write(struct annotation_line *al, struct annotation *notes,
> > > -			    struct annotation_write_ops *ops)
> > > +			    struct annotation_write_ops *wops,
> > > +			    struct annotation_options *opts)
> > >  {
> > > -	__annotation_line__write(al, notes, ops->first_line, ops->current_entry,
> > > -				 ops->change_color, ops->width, ops->obj,
> > > -				 ops->set_color, ops->set_percent_color,
> > > -				 ops->set_jumps_percent_color, ops->printf,
> > > -				 ops->write_graph);
> > > +	__annotation_line__write(al, notes, wops->first_line, wops->current_entry,
> > > +				 wops->change_color, wops->width, wops->obj,
> > > +				 opts->percent_type,
> > > +				 wops->set_color, wops->set_percent_color,
> > > +				 wops->set_jumps_percent_color, wops->printf,
> > > +				 wops->write_graph);
> > 
> > This doesn't look good.  Why not just passing a pointer to wops
> > instead of each fields separately?
> 
> yep, my thoughts exactly when I saw this ;-) we probably had some

But then, while this is a valid observation, it is not related to this
patchkit, that is just adding an extra config variable, percent_type, at
some point one can try to shorten that function signature, looking at
why it was done this way originally to see if there was any reason or if
its just something to improve by shortening the function signature.

Applying Jiri's patch,

> other caller..  however I only wanted to add one more param ;-)

Right
 
> I'll check what we can do with this in v2

I'm going thru v2 already, so far its an uncontrovertial, trivial, so
I think you better just wait a teeny bit for this to be applied and then
get on over with followup patches, ok?

- Arnaldo

  reply	other threads:[~2018-08-06 15:22 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-04 13:05 [PATCH 00/20] perf annotate: Make symbol__annotate_fprintf2 local Jiri Olsa
2018-08-04 13:05 ` [PATCH 01/20] " Jiri Olsa
2018-08-06 15:00   ` Arnaldo Carvalho de Melo
2018-08-18 11:25   ` [tip:perf/urgent] perf annotate: Make symbol__annotate_fprintf2() local tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 02/20] perf annotate: Make annotation_line__max_percent static Jiri Olsa
2018-08-06 15:01   ` Arnaldo Carvalho de Melo
2018-08-18 11:26   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 03/20] perf annotate: Get rid of annotation__scnprintf_samples_period Jiri Olsa
2018-08-06 15:07   ` Arnaldo Carvalho de Melo
2018-08-18 11:26   ` [tip:perf/urgent] perf annotate: Get rid of annotation__scnprintf_samples_period() tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 04/20] perf annotate: Rename struct annotation_line::samples* to data* Jiri Olsa
2018-08-06 15:08   ` Arnaldo Carvalho de Melo
2018-08-18 11:27   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 05/20] perf annotate: Rename local sample variables to data Jiri Olsa
2018-08-06 15:09   ` Arnaldo Carvalho de Melo
2018-08-18 11:27   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 06/20] perf annotate: Rename hist to sym_hist in annotation__calc_percent Jiri Olsa
2018-08-06 15:10   ` Arnaldo Carvalho de Melo
2018-08-18 11:28   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 07/20] perf annotate: Loop group events directly " Jiri Olsa
2018-08-06 15:12   ` Arnaldo Carvalho de Melo
2018-08-18 11:28   ` [tip:perf/urgent] perf annotate: Loop group events directly in annotation__calc_percent() tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 08/20] perf annotate: Switch struct annotation_data::percent to array Jiri Olsa
2018-08-06 15:14   ` Arnaldo Carvalho de Melo
2018-08-18 11:29   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 09/20] perf annotate: Add PERCENT_HITS_GLOBAL percent value Jiri Olsa
2018-08-06 15:15   ` Arnaldo Carvalho de Melo
2018-08-18 11:29   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 10/20] perf annotate: Add PERCENT_PERIOD_LOCAL " Jiri Olsa
2018-08-06 15:16   ` Arnaldo Carvalho de Melo
2018-08-18 11:30   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 11/20] perf annotate: Add PERCENT_PERIOD_GLOBAL " Jiri Olsa
2018-08-06 15:16   ` Arnaldo Carvalho de Melo
2018-08-18 11:30   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 12/20] perf annotate: Add percent_type to struct annotation_options Jiri Olsa
2018-08-06 15:17   ` Arnaldo Carvalho de Melo
2018-08-18 11:31   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 13/20] perf annotate: Pass struct annotation_options to symbol__calc_lines Jiri Olsa
2018-08-06 15:18   ` Arnaldo Carvalho de Melo
2018-08-18 11:32   ` [tip:perf/urgent] perf annotate: Pass struct annotation_options to symbol__calc_lines() tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 14/20] perf annotate: Pass struct annotation_options to map_symbol__annotation_dump Jiri Olsa
2018-08-06 13:45   ` Namhyung Kim
2018-08-06 14:24     ` Jiri Olsa
2018-08-06 15:22       ` Arnaldo Carvalho de Melo [this message]
2018-08-06 15:29         ` Jiri Olsa
2018-08-18 11:32   ` [tip:perf/urgent] perf annotate: Pass 'struct annotation_options' to map_symbol__annotation_dump() tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 15/20] perf annotate: Pass browser percent_type in annotate_browser__calc_percent Jiri Olsa
2018-08-06 15:23   ` Arnaldo Carvalho de Melo
2018-08-18 11:33   ` [tip:perf/urgent] perf annotate: Pass browser percent_type in annotate_browser__calc_percent() tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 16/20] perf annotate: Add support to togle percent type Jiri Olsa
2018-08-06 18:24   ` Arnaldo Carvalho de Melo
2018-08-18 11:33   ` [tip:perf/urgent] perf annotate: Add support to toggle " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 17/20] perf annotate: Make local period the default " Jiri Olsa
2018-08-06 18:26   ` Arnaldo Carvalho de Melo
2018-08-18 11:34   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 18/20] perf annotate: Display percent type in stdio output Jiri Olsa
2018-08-06 18:27   ` Arnaldo Carvalho de Melo
2018-08-18 11:34   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 19/20] perf annotate: Add --percent-type option Jiri Olsa
2018-08-06 13:49   ` Namhyung Kim
2018-08-06 14:26     ` Jiri Olsa
2018-08-06 18:33       ` Arnaldo Carvalho de Melo
2018-08-06 18:32   ` Arnaldo Carvalho de Melo
2018-08-18 11:35   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2018-08-04 13:05 ` [PATCH 20/20] perf report: " Jiri Olsa
2018-08-06 18:48   ` Arnaldo Carvalho de Melo
2018-08-18 11:35   ` [tip:perf/urgent] " tip-bot for Jiri Olsa

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=20180806152219.GA14525@kernel.org \
    --to=acme@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=dsahern@gmail.com \
    --cc=eranian@google.com \
    --cc=jolsa@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --subject='Re: [PATCH 14/20] perf annotate: Pass struct annotation_options to map_symbol__annotation_dump' \
    /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

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox