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

[Damien Lespiau <damien.lespiau@intel.com>]

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.

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?

-- 
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx