On Mon, Oct 13, 2014 at 02:05:34PM +0200, Paolo Bonzini wrote: > Il 24/09/2014 17:21, Benoît Canet ha scritto: > > The module takes care of computing minimal and maximal > > values over the time slice duration. > > The code looks good, just two comments: > > > +/* Get the average value > > + * > > + * @ta: the timed average structure used > > + * @ret: the average value > > + */ > > +uint64_t timed_average_avg(TimedAverage *ta) > > +{ > > + Window *w; > > + check_expirations(ta); > > + > > + w = current_window(ta); > > + > > + if (w->count) { > > + return w->sum / w->count; > > First, do we want this to return double? > > Second, this will return the min/max/avg in an unknown amount of time > between period/2 and period---on average period*3/4, e.g. 0.75 seconds > for a period equal to one second. > > Would it make sense to tweak the TimedAverage period to be higher, e.g. > 1.33 seconds/80 seconds/80 minutes, so that the _average_ period is 1 > second/1 minute/1 hour? > > This only applies to how the code is used, not to TimedAverage itself; > hence, feel free to post the patch with Reviewed-by once > timed_average_avg's return type is changed to a double.
This would require either changing _init period units or making it a float or baking the 1.33 ratio in timed average. Which one would you prefer ? Best regards Benoît Thanks for reviewing. > > Paolo > > > + } > > + > > + return 0; > > +} > > + > > +/* Get the maximum value > > + * > > + * @ta: the timed average structure used > > + * @ret: the maximal value > > + */ > > +uint64_t timed_average_max(TimedAverage *ta) > > +{ > > + check_expirations(ta); > > + return current_window(ta)->max; > > +} > > >