Re: [PATCH 09/21] perf ui/hist: Add support to accumulated hist stat

[Namhyung Kim <namhyung@kernel.org>]

On Sun, 5 Jan 2014 18:31:13 +0100, Jiri Olsa wrote:
> On Tue, Dec 24, 2013 at 05:22:15PM +0900, Namhyung Kim wrote:
>> +#define __HPP_COLOR_ACC_PERCENT_FN(_type, _field)				\
>> +static u64 he_get_acc_##_field(struct hist_entry *he)				\
>> +{										\
>> +	return he->stat_acc->_field;						\
>> +}										\
>> +										\
>> +static int hpp__color_acc_##_type(struct perf_hpp_fmt *fmt __maybe_unused,	\
>> +			      struct perf_hpp *hpp, struct hist_entry *he) 	\
>> +{										\
>> +	return __hpp__fmt(hpp, he, he_get_acc_##_field, " %6.2f%%",		\
>> +			  (hpp_snprint_fn)percent_color_snprintf, true);	\
>> +}
>> +
>> +#define __HPP_ENTRY_ACC_PERCENT_FN(_type, _field)				\
>> +static int hpp__entry_acc_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,	\
>> +			      struct perf_hpp *hpp, struct hist_entry *he) 	\
>> +{										\
>> +	const char *fmt = symbol_conf.field_sep ? " %.2f" : " %6.2f%%";		\
>> +	return __hpp__fmt(hpp, he, he_get_acc_##_field, fmt,			\
>> +			  scnprintf, true);					\
>> +}
>> +
>>  #define __HPP_ENTRY_RAW_FN(_type, _field)					\
>>  static u64 he_get_raw_##_field(struct hist_entry *he)				\
>>  {										\
>> @@ -148,17 +170,25 @@ __HPP_WIDTH_FN(_type, _min_width, _unit_width)				\
>>  __HPP_COLOR_PERCENT_FN(_type, _field)					\
>>  __HPP_ENTRY_PERCENT_FN(_type, _field)
>>  
>> +#define HPP_PERCENT_ACC_FNS(_type, _str, _field, _min_width, _unit_width)\
>> +__HPP_HEADER_FN(_type, _str, _min_width, _unit_width)			\
>> +__HPP_WIDTH_FN(_type, _min_width, _unit_width)				\
>> +__HPP_COLOR_ACC_PERCENT_FN(_type, _field)				\
>> +__HPP_ENTRY_ACC_PERCENT_FN(_type, _field)
>
> how about add 'stat|stat_acc' parameter to the HPP_PERCENT_FNS
> macro and use it like:
>
> #define __HPP_COLOR_PERCENT_FN(_type, _field, _stat)                            \
> static u64 he_get_##_stat##_field(struct hist_entry *he)                        \
> {                                                                               \
>         return he->##_stat._field;                                              \
> }                                                                               \
>
>
> and use 'he_get_##_stat##_field' in __HPP_ENTRY_PERCENT_FN
>
> This way you dont need to duplicate the code, that differs
> only in the used stat data.. but it's possible I missed
> something ;-)

The problem is that he->stat is a in-struct data but ->stat_acc is
dynamically allocated.  So it should be 

  "he->stat. ## _field"
           ~
vs.

  "he->stat_acc-> ## _field"
               ~~
>
> btw just noticed __HPP_COLOR_PERCENT_FN does not use _type

???  It's used in hpp__color_##_type().


Thanks,
Namhyung