diff mbox

[i-g-t,1/3] stats: Allow the underlying arrays to grow at will

Message ID 1435424869-17455-2-git-send-email-damien.lespiau@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lespiau, Damien June 27, 2015, 5:07 p.m. UTC
Chris mentioned he wanted to be able to measure a variable "for one
second" and use igt_stats to store them. That's one case where we don't
know the number of data points upfront.

We should really support that, so here it is.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/igt_stats.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 4 deletions(-)

Comments

Chris Wilson June 27, 2015, 5:15 p.m. UTC | #1
On Sat, Jun 27, 2015 at 06:07:47PM +0100, Damien Lespiau wrote:
> Chris mentioned he wanted to be able to measure a variable "for one
> second" and use igt_stats to store them. That's one case where we don't
> know the number of data points upfront.
> 
> We should really support that, so here it is.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  lib/igt_stats.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_stats.c b/lib/igt_stats.c
> index caf3f37..cd97ab0 100644
> --- a/lib/igt_stats.c
> +++ b/lib/igt_stats.c
> @@ -63,6 +63,41 @@
>   * ]|
>   */
>  
> +static unsigned int get_new_capacity(int need)
> +{
> +	unsigned int new_capacity;
> +
> +	/* taken from Python's list */
> +	new_capacity = (need >> 6) + (need < 9 ? 3 : 6);
> +	new_capacity += need;
> +
> +	return new_capacity;
> +}
> +
> +static void igt_stats_ensure_capacity(igt_stats_t *stats,
> +				      unsigned int n_additional_values)
> +{
> +	unsigned int new_n_values = stats->n_values + n_additional_values;
> +	unsigned int new_capacity;
> +
> +	if (new_n_values <= stats->capacity)
> +		return;
> +
> +	new_capacity = get_new_capacity(new_n_values);
> +	stats->values = realloc(stats->values,
> +				sizeof(*stats->values) * new_capacity);
> +	igt_assert(stats->values);
> +
> +	stats->capacity = new_capacity;
> +
> +	if (!stats->sorted)
> +		return;
> +
> +	stats->sorted = realloc(stats->sorted,
> +				sizeof(*stats->values) * new_capacity);
> +	igt_assert(stats->sorted);

There isn't any point in preserving the sorted array when pushing new
values. Might as well just free here and allocate again when required.
-Chris
Lespiau, Damien June 27, 2015, 6:06 p.m. UTC | #2
On Sat, Jun 27, 2015 at 06:15:42PM +0100, Chris Wilson wrote:
> > +static void igt_stats_ensure_capacity(igt_stats_t *stats,
> > +				      unsigned int n_additional_values)
> > +{
> > +	unsigned int new_n_values = stats->n_values + n_additional_values;
> > +	unsigned int new_capacity;
> > +
> > +	if (new_n_values <= stats->capacity)
> > +		return;
> > +
> > +	new_capacity = get_new_capacity(new_n_values);
> > +	stats->values = realloc(stats->values,
> > +				sizeof(*stats->values) * new_capacity);
> > +	igt_assert(stats->values);
> > +
> > +	stats->capacity = new_capacity;
> > +
> > +	if (!stats->sorted)
> > +		return;
> > +
> > +	stats->sorted = realloc(stats->sorted,
> > +				sizeof(*stats->values) * new_capacity);
> > +	igt_assert(stats->sorted);
> 
> There isn't any point in preserving the sorted array when pushing new
> values. Might as well just free here and allocate again when required.

Indeed, push the whole thing so far with this change.
diff mbox

Patch

diff --git a/lib/igt_stats.c b/lib/igt_stats.c
index caf3f37..cd97ab0 100644
--- a/lib/igt_stats.c
+++ b/lib/igt_stats.c
@@ -63,6 +63,41 @@ 
  * ]|
  */
 
+static unsigned int get_new_capacity(int need)
+{
+	unsigned int new_capacity;
+
+	/* taken from Python's list */
+	new_capacity = (need >> 6) + (need < 9 ? 3 : 6);
+	new_capacity += need;
+
+	return new_capacity;
+}
+
+static void igt_stats_ensure_capacity(igt_stats_t *stats,
+				      unsigned int n_additional_values)
+{
+	unsigned int new_n_values = stats->n_values + n_additional_values;
+	unsigned int new_capacity;
+
+	if (new_n_values <= stats->capacity)
+		return;
+
+	new_capacity = get_new_capacity(new_n_values);
+	stats->values = realloc(stats->values,
+				sizeof(*stats->values) * new_capacity);
+	igt_assert(stats->values);
+
+	stats->capacity = new_capacity;
+
+	if (!stats->sorted)
+		return;
+
+	stats->sorted = realloc(stats->sorted,
+				sizeof(*stats->values) * new_capacity);
+	igt_assert(stats->sorted);
+}
+
 /**
  * igt_stats_init:
  * @stats: An #igt_stats_t instance
@@ -78,9 +113,7 @@  void igt_stats_init(igt_stats_t *stats, unsigned int capacity)
 {
 	memset(stats, 0, sizeof(*stats));
 
-	stats->values = calloc(capacity, sizeof(*stats->values));
-	igt_assert(stats->values);
-	stats->capacity = capacity;
+	igt_stats_ensure_capacity(stats, capacity);
 
 	stats->min = U64_MAX;
 	stats->max = 0;
@@ -156,7 +189,8 @@  void igt_stats_set_population(igt_stats_t *stats, bool full_population)
  */
 void igt_stats_push(igt_stats_t *stats, uint64_t value)
 {
-	igt_assert(stats->n_values < stats->capacity);
+	igt_stats_ensure_capacity(stats, 1);
+
 	stats->values[stats->n_values++] = value;
 
 	stats->mean_variance_valid = false;
@@ -181,6 +215,8 @@  void igt_stats_push_array(igt_stats_t *stats,
 {
 	unsigned int i;
 
+	igt_stats_ensure_capacity(stats, n_values);
+
 	for (i = 0; i < n_values; i++)
 		igt_stats_push(stats, values[i]);
 }
@@ -240,6 +276,14 @@  static void igt_stats_ensure_sorted_values(igt_stats_t *stats)
 		return;
 
 	if (!stats->sorted) {
+		/*
+		 * We could always allocate sorted, but doing it lazily will
+		 * consume a bit less memory if the user doesn't use any of the
+		 * functions needing to sort the data points.
+		 *
+		 * igt_stats_ensure_capacity() will take care of reallocating
+		 * sorted when needed after this initial allocation.
+		 */
 		stats->sorted = calloc(stats->capacity, sizeof(*stats->values));
 		igt_assert(stats->sorted);
 	}