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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
