On Fri, 30 Nov 2012 10:52:15 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 30, 2012 at 02:29:43PM +0100, Jiri Olsa escreveu:
>> On Thu, Nov 29, 2012 at 03:38:40PM +0900, Namhyung Kim wrote:
>> > +#define __HPP_COLOR_PERCENT_FN(_type, _field)                             
>> >         \
>> > +static int hpp__color_##_type(struct perf_hpp *hpp, struct hist_entry 
>> > *he)        \
>> > +{                                                                         
>> > \
>> > +  int ret;                                                                
>> > \
>> > +  double percent = 0.0;                                                   
>> > \
>> > +  struct hists *hists = he->hists;                                        
>> > \
>> > +                                                                          
>> > \
>> > +  if (hists->stats.total_period)                                          
>> > \
>> > +          percent = 100.0 * he->stat._field / hists->stats.total_period;  
>> > \
>> > +                                                                          
>> > \
>> > +  ret = percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent); 
>> > \
>> > +                                                                          
>> > \
>> > +  if (symbol_conf.event_group) {                                          
>> > \
>> > +          int i;                                                          
>> > \
>> > +          struct perf_evsel *evsel = hists_to_evsel(hists);               
>> > \
>> > +          struct hist_entry *pair;                                        
>> > \
>> > +          int nr_members = evsel->nr_members;                             
>> > \
>> > +          double *percents;                                               
>> > \
>> > +                                                                          
>> > \
>> > +          if (nr_members <= 1)                                            
>> > \
>> > +                  return ret;                                             
>> > \
>> > +                                                                          
>> > \
>> > +          percents = zalloc(sizeof(*percents) * nr_members);              
>> > \
>> > +          if (percents == NULL) {                                         
>> > \
>> > +                  pr_warning("Not enough memory!\n");                     
>> > \
>> > +                  return ret;                                             
>> > \
>> > +          }                                                               
>> > \
>
> Why do we need to zalloc this percents array?
>
>> > +          list_for_each_entry(pair, &he->pairs.head, pairs.node) {        
>> > \
>> > +                  u64 period = pair->stat._field;                         
>> > \
>> > +                  u64 total = pair->hists->stats.total_period;            
>> > \
>> > +                                                                          
>> > \
>> > +                  if (!total)                                             
>> > \
>> > +                          continue;                                       
>> > \
>> > +                                                                          
>> > \
>> > +                  evsel = hists_to_evsel(pair->hists);                    
>> > \
>> > +                  i = perf_evsel__group_idx(evsel);                       
>> > \
>> > +                  percents[i] = 100.0 * period / total;                   
>> > \
>
> Why not use percent_color_snprintf here, using some "%*s" thing that
> uses i to position the output in the right column? This way the
> following loop can be ditched as well. No?

It's because it's possible that an entry didn't have pairs for every
group member.  Say, there's a group consists of 3 members (1 leader + 2
member).  It's quite legitimate that a leader hist entry has just one
pair for a member and miss another.  So I allocated a zero-filled array,
and filled what it has, and print them all in the for loop below.

Thanks,
Namhyung

>
>> > +          }                                                               
>> > \
>> > +                                                                          
>> > \
>> > +          for (i = 1; i < nr_members; i++) {                              
>> > \
>> > +                  ret += percent_color_snprintf(hpp->buf + ret,           
>> > \
>> > +                                                hpp->size - ret,          
>> > \
>> > +                                                " %6.2f%%", percents[i]); 
>> > \
>> > +          }                                                               
>> > \
>> > +          free(percents);                                                 
>> > \
>> > +  }                                                                       
>> > \
>> > +  return ret;                                                             
>> > \
>
> - Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to