Re: PATCH: decreasing memory needlessly consumed by array_agg

On Wed, 2015-01-28 at 23:25 +0100, Tomas Vondra wrote:
> 3) moves the assert into the 'if (release)' branch

You missed one, but I got it.

> 4) includes the comments proposed by Ali Akbar in his reviews
>
> Warnings at makeArrayResult/makeMdArrayResult about freeing memory
> with private subcontexts.

I also edited the comments substantially.

> Regarding the performance impact of decreasing the size of the
> preallocated array from 64 to just 8 elements, I tried this.
>
> CREATE TABLE test AS
> SELECT mod(i,100000) a, i FROM generate_series(1,64*100000) s(i);
>
> SELECT a, array_agg(i) AS x FRM test GROUP BY 1;
>
> or actually (to minimize transfer overhead):
>
> SELECT COUNT(x) FROM (
> SELECT a, array_agg(i) AS x FRM test GROUP BY 1
> ) foo;

That's actually a bogus test -- array_agg is never executed. See the
test script I used (attached). Many small groups has a significant
improvement with your patch (median 167ms unpatched, 125ms patched), and
the one large group is not measurably different (median 58ms for both).

The test script uses a small dataset of 100K tuples, because 1M tuples
will run out of memory for small groups without the patch.

Committed.

Regards,
Jeff Davis