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

With this API, i think we should make it clear if we call initArrayResult with subcontext=false, we can't call makeArrayResult, but we must use makeMdArrayResult directly.

Or better, we can modify makeArrayResult to release according to astate->private_cxt:

@@ -4742,7 +4742,7 @@ makeArrayResult(ArrayBuildState *astate,
    dims[0] = astate->nelems;
    lbs[0] = 1;
 
-   return makeMdArrayResult(astate, ndims, dims, lbs, rcontext, true);
+   return makeMdArrayResult(astate, ndims, dims, lbs, rcontext, astate->private_cxt);

Or else we implement what you suggest below (more comments below):

Thinking about the 'release' flag a bit more - maybe we could do this
instead:

    if (release && astate->private_cxt)
        MemoryContextDelete(astate->mcontext);
    else if (release)
    {
        pfree(astate->dvalues);
        pfree(astate->dnulls);
        pfree(astate);
    }

i.e. either destroy the whole context if possible, and just free the
memory when using a shared memory context. But I'm afraid this would
penalize the shared memory context, because that's intended for cases
where all the build states coexist in parallel and then at some point
are all converted into a result and thrown away. Adding pfree() calls is
no improvement here, and just wastes cycles.

As per Tom's comment, i'm using "parent memory context" instead of "shared memory context" below.

In the future, if some code writer decided to use subcontext=false, to save memory in cases where there are many array accumulation, and the parent memory context is long-living, current code can cause memory leak. So i think we should implement your suggestion (pfreeing astate), and warn the implication in the API comment. The API user must choose between release=true, wasting cycles but preventing memory leak, or managing memory from the parent memory context.

In one possible use case, for efficiency maybe the caller will create a special parent memory context for all array accumulation. Then uses makeArrayResult* with release=false, and in the end releasing the parent memory context once for all.