Re: PATCH: decreasing memory needlessly consumed by array_agg - Mailing list pgsql-hackers

From Tom Lane
Subject Re: PATCH: decreasing memory needlessly consumed by array_agg
Date
Msg-id 24480.1396378565@sss.pgh.pa.us
Whole thread Raw
In response to Re: PATCH: decreasing memory needlessly consumed by array_agg  (Tomas Vondra <tv@fuzzy.cz>)
Responses Re: PATCH: decreasing memory needlessly consumed by array_agg  (Tomas Vondra <tv@fuzzy.cz>)
List pgsql-hackers
Tomas Vondra <tv@fuzzy.cz> writes:
> On 1.4.2014 19:08, Tom Lane wrote:
>> Actually, though, the patch as given outright breaks things for both
>> the grouped and ungrouped cases, because the aggregate no longer
>> releases memory when it's done. That's going to result in memory
>> bloat not savings, in any situation where the aggregate is executed
>> repeatedly.

> Looking at array_agg_finalfn (which is the final function for
> array_agg), I see it does this:

>     /*
>      * Make the result.  We cannot release the ArrayBuildState because
>      * sometimes aggregate final functions are re-executed.  Rather, it
>      * is nodeAgg.c's responsibility to reset the aggcontext when it's
>      * safe to do so.
>      */
>     result = makeMdArrayResult(state, 1, dims, lbs,
>                                CurrentMemoryContext,
>                                false);

> i.e. it sets release=false. So I fail to see how the current code
> behaves differently from the patch?

You're conveniently ignoring the callers that set release=true.
Reverse engineering a query that exhibits memory bloat is left
as an exercise for the reader (but in a quick look, I'll bet
ARRAY_SUBLINK subplans are one locus for problems).

It's possible that it'd work to use a subcontext only if release=true;
I've not dug through the code enough to convince myself of that.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Tomas Vondra
Date:
Subject: Re: PATCH: decreasing memory needlessly consumed by array_agg
Next
From: Tomas Vondra
Date:
Subject: Re: PATCH: decreasing memory needlessly consumed by array_agg