[Intel-gfx] [PATCH i-g-t 17/18] stats: Add support for quartiles (and thus median)

[Chris Wilson]

On Sat, Jun 27, 2015 at 04:52:56PM +0100, Damien Lespiau wrote:
> On Sat, Jun 27, 2015 at 04:15:41PM +0100, Chris Wilson wrote:
> > > +static void igt_stats_ensure_sorted_values(igt_stats_t *stats)
> > > +{
> > > +	if (stats->sorted_array_valid)
> > > +		return;
> > > +
> > > +	if (!stats->sorted) {
> > > +		stats->sorted = calloc(stats->capacity, sizeof(*stats->values));
> > > +		igt_assert(stats->sorted);
> > > +	}
> > 
> > Since sorted_array_valid = false on igt_stats_push, but we only allocate
> > for the first call to sort, there's no safeguard against
> > 
> >   igt_stats_push();
> >   igt_stats_get_median();
> >   igt_stats_push();
> >   igt_stats_get_median();
> > 
> > exploding.
> 
> I believe it's fine (and I just sent a test telling the same story, but
> well, overflowing an small heap allocated array can often be "fine" with
> an overallocation from malloc() rounding up the size to git in a bin),
> as the size of the dataset is an invariant (I didn't implement your
> suggestion to grow the array at push() time). We allocate the full
> capacity here, not just the current n_values.

Ah, didn't realise that the capacity was fixed.
 
> I did have a think about a growing ->values array, but I think we know
> the number of data points we want upfront most of the time (if not
> always) when going to do some measurements?

No, often times I want to run a benchmark for x seconds and then look at
the spread of samples.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre