On 22.2.2015 02:38, Jeff Davis wrote: > 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.
Oh, thanks! > >> 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. Thanks. >> 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. Really? How could that happen when the result of array_agg() is passed to the COUNT()? Also, how could that allocate huge amounts of memory and get killed by OOM, which happens easily with this query? > 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). Yup, that's expected. With a lot of small groups the improvement may easily be much higher, but this is a conservative test. > > 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. Cool. Thanks once more. regards -- Tomas Vondra http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers