Skip to content

Commit b419865

Browse files
jeff-davisjeff-davis
committed
In array_agg(), don't create a new context for every group.
Previously, each new array created a new memory context that started out at 8kB. This is incredibly wasteful when there are lots of small groups of just a few elements each. Change initArrayResult() and friends to accept a "subcontext" argument to indicate whether the caller wants the ArrayBuildState allocated in a new subcontext or not. If not, it can no longer be released separately from the rest of the memory context. Fixes bug report by Frank van Vugt on 2013-10-19. Tomas Vondra. Reviewed by Ali Akbar, Tom Lane, and me.
1 parent e9fd554 commit b419865

File tree

6 files changed

+92
-44
lines changed

6 files changed

+92
-44
lines changed

‎src/backend/executor/nodeSubplan.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ ExecScanSubPlan(SubPlanState *node,
262262
/* Initialize ArrayBuildStateAny in caller's context, if needed */
263263
if (subLinkType == ARRAY_SUBLINK)
264264
astate = initArrayResultAny(subplan->firstColType,
265-
CurrentMemoryContext);
265+
CurrentMemoryContext, true);
266266

267267
/*
268268
* We are probably in a short-lived expression-evaluation context. Switch
@@ -964,7 +964,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
964964
/* Initialize ArrayBuildStateAny in caller's context, if needed */
965965
if (subLinkType == ARRAY_SUBLINK)
966966
astate = initArrayResultAny(subplan->firstColType,
967-
CurrentMemoryContext);
967+
CurrentMemoryContext, true);
968968

969969
/*
970970
* Must switch to per-query memory context.

‎src/backend/utils/adt/array_userfuncs.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,8 +520,13 @@ array_agg_transfn(PG_FUNCTION_ARGS)
520520
elog(ERROR, "array_agg_transfn called in non-aggregate context");
521521
}
522522

523-
state = PG_ARGISNULL(0) ? NULL : (ArrayBuildState *) PG_GETARG_POINTER(0);
523+
if (PG_ARGISNULL(0))
524+
state = initArrayResult(arg1_typeid, aggcontext, false);
525+
else
526+
state = (ArrayBuildState *) PG_GETARG_POINTER(0);
527+
524528
elem = PG_ARGISNULL(1) ? (Datum) 0 : PG_GETARG_DATUM(1);
529+
525530
state = accumArrayResult(state,
526531
elem,
527532
PG_ARGISNULL(1),
@@ -595,7 +600,12 @@ array_agg_array_transfn(PG_FUNCTION_ARGS)
595600
elog(ERROR, "array_agg_array_transfn called in non-aggregate context");
596601
}
597602

598-
state = PG_ARGISNULL(0) ? NULL : (ArrayBuildStateArr *) PG_GETARG_POINTER(0);
603+
604+
if (PG_ARGISNULL(0))
605+
state = initArrayResultArr(arg1_typeid, InvalidOid, aggcontext, false);
606+
else
607+
state = (ArrayBuildStateArr *) PG_GETARG_POINTER(0);
608+
599609
state = accumArrayResultArr(state,
600610
PG_GETARG_DATUM(1),
601611
PG_ARGISNULL(1),

‎src/backend/utils/adt/arrayfuncs.c

Lines changed: 71 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4650,6 +4650,7 @@ array_insert_slice(ArrayType *destArray,
46504650
*
46514651
* element_type is the array element type (must be a valid array element type)
46524652
* rcontext is where to keep working state
4653+
* subcontext is a flag determining whether to use a separate memory context
46534654
*
46544655
* Note: there are two common schemes for using accumArrayResult().
46554656
* In the older scheme, you start with a NULL ArrayBuildState pointer, and
@@ -4659,24 +4660,39 @@ array_insert_slice(ArrayType *destArray,
46594660
* once per element. In this scheme you always end with a non-NULL pointer
46604661
* that you can pass to makeArrayResult; you get an empty array if there
46614662
* were no elements. This is preferred if an empty array is what you want.
4663+
*
4664+
* It's possible to choose whether to create a separate memory context for the
4665+
* array build state, or whether to allocate it directly within rcontext.
4666+
*
4667+
* When there are many concurrent small states (e.g. array_agg() using hash
4668+
* aggregation of many small groups), using a separate memory context for each
4669+
* one may result in severe memory bloat. In such cases, use the same memory
4670+
* context to initialize all such array build states, and pass
4671+
* subcontext=false.
4672+
*
4673+
* In cases when the array build states have different lifetimes, using a
4674+
* single memory context is impractical. Instead, pass subcontext=true so that
4675+
* the array build states can be freed individually.
46624676
*/
46634677
ArrayBuildState *
4664-
initArrayResult(Oid element_type, MemoryContext rcontext)
4678+
initArrayResult(Oid element_type, MemoryContext rcontext, bool subcontext)
46654679
{
46664680
ArrayBuildState *astate;
4667-
MemoryContext arr_context;
4681+
MemoryContext arr_context = rcontext;
46684682

46694683
/* Make a temporary context to hold all the junk */
4670-
arr_context = AllocSetContextCreate(rcontext,
4671-
"accumArrayResult",
4672-
ALLOCSET_DEFAULT_MINSIZE,
4673-
ALLOCSET_DEFAULT_INITSIZE,
4674-
ALLOCSET_DEFAULT_MAXSIZE);
4684+
if (subcontext)
4685+
arr_context = AllocSetContextCreate(rcontext,
4686+
"accumArrayResult",
4687+
ALLOCSET_DEFAULT_MINSIZE,
4688+
ALLOCSET_DEFAULT_INITSIZE,
4689+
ALLOCSET_DEFAULT_MAXSIZE);
46754690

46764691
astate = (ArrayBuildState *)
46774692
MemoryContextAlloc(arr_context, sizeof(ArrayBuildState));
46784693
astate->mcontext = arr_context;
4679-
astate->alen = 64; /* arbitrary starting array size */
4694+
astate->private_cxt = subcontext;
4695+
astate->alen = (subcontext ? 64 : 8); /* arbitrary starting array size */
46804696
astate->dvalues = (Datum *)
46814697
MemoryContextAlloc(arr_context, astate->alen * sizeof(Datum));
46824698
astate->dnulls = (bool *)
@@ -4710,7 +4726,7 @@ accumArrayResult(ArrayBuildState *astate,
47104726
if (astate == NULL)
47114727
{
47124728
/* First time through --- initialize */
4713-
astate = initArrayResult(element_type, rcontext);
4729+
astate = initArrayResult(element_type, rcontext, true);
47144730
}
47154731
else
47164732
{
@@ -4757,6 +4773,9 @@ accumArrayResult(ArrayBuildState *astate,
47574773
/*
47584774
* makeArrayResult - produce 1-D final result of accumArrayResult
47594775
*
4776+
* Note: only releases astate if it was initialized within a separate memory
4777+
* context (i.e. using subcontext=true when calling initArrayResult).
4778+
*
47604779
* astate is working state (must not be NULL)
47614780
* rcontext is where to construct result
47624781
*/
@@ -4773,7 +4792,8 @@ makeArrayResult(ArrayBuildState *astate,
47734792
dims[0] = astate->nelems;
47744793
lbs[0] = 1;
47754794

4776-
return makeMdArrayResult(astate, ndims, dims, lbs, rcontext, true);
4795+
return makeMdArrayResult(astate, ndims, dims, lbs, rcontext,
4796+
astate->private_cxt);
47774797
}
47784798

47794799
/*
@@ -4782,6 +4802,11 @@ makeArrayResult(ArrayBuildState *astate,
47824802
* beware: no check that specified dimensions match the number of values
47834803
* accumulated.
47844804
*
4805+
* Note: if the astate was not initialized within a separate memory context
4806+
* (that is, initArrayResult was called with subcontext=false), then using
4807+
* release=true is illegal. Instead, release astate along with the rest of its
4808+
* context when appropriate.
4809+
*
47854810
* astate is working state (must not be NULL)
47864811
* rcontext is where to construct result
47874812
* release is true if okay to release working state
@@ -4814,7 +4839,10 @@ makeMdArrayResult(ArrayBuildState *astate,
48144839

48154840
/* Clean up all the junk */
48164841
if (release)
4842+
{
4843+
Assert(astate->private_cxt);
48174844
MemoryContextDelete(astate->mcontext);
4845+
}
48184846

48194847
return PointerGetDatum(result);
48204848
}
@@ -4831,26 +4859,42 @@ makeMdArrayResult(ArrayBuildState *astate,
48314859
* initArrayResultArr - initialize an empty ArrayBuildStateArr
48324860
*
48334861
* array_type is the array type (must be a valid varlena array type)
4834-
* element_type is the type of the array's elements
4862+
* element_type is the type of the array's elements (lookup if InvalidOid)
48354863
* rcontext is where to keep working state
4864+
* subcontext is a flag determining whether to use a separate memory context
48364865
*/
48374866
ArrayBuildStateArr *
4838-
initArrayResultArr(Oid array_type, Oid element_type, MemoryContext rcontext)
4867+
initArrayResultArr(Oid array_type, Oid element_type, MemoryContext rcontext,
4868+
bool subcontext)
48394869
{
48404870
ArrayBuildStateArr *astate;
4841-
MemoryContext arr_context;
4871+
MemoryContext arr_context = rcontext; /* by default use the parent ctx */
4872+
4873+
/* Lookup element type, unless element_type already provided */
4874+
if (! OidIsValid(element_type))
4875+
{
4876+
element_type = get_element_type(array_type);
4877+
4878+
if (!OidIsValid(element_type))
4879+
ereport(ERROR,
4880+
(errcode(ERRCODE_DATATYPE_MISMATCH),
4881+
errmsg("data type %s is not an array type",
4882+
format_type_be(array_type))));
4883+
}
48424884

48434885
/* Make a temporary context to hold all the junk */
4844-
arr_context = AllocSetContextCreate(rcontext,
4845-
"accumArrayResultArr",
4846-
ALLOCSET_DEFAULT_MINSIZE,
4847-
ALLOCSET_DEFAULT_INITSIZE,
4848-
ALLOCSET_DEFAULT_MAXSIZE);
4886+
if (subcontext)
4887+
arr_context = AllocSetContextCreate(rcontext,
4888+
"accumArrayResultArr",
4889+
ALLOCSET_DEFAULT_MINSIZE,
4890+
ALLOCSET_DEFAULT_INITSIZE,
4891+
ALLOCSET_DEFAULT_MAXSIZE);
48494892

48504893
/* Note we initialize all fields to zero */
48514894
astate = (ArrayBuildStateArr *)
48524895
MemoryContextAllocZero(arr_context, sizeof(ArrayBuildStateArr));
48534896
astate->mcontext = arr_context;
4897+
astate->private_cxt = subcontext;
48544898

48554899
/* Save relevant datatype information */
48564900
astate->array_type = array_type;
@@ -4897,21 +4941,9 @@ accumArrayResultArr(ArrayBuildStateArr *astate,
48974941
arg = DatumGetArrayTypeP(dvalue);
48984942

48994943
if (astate == NULL)
4900-
{
4901-
/* First time through --- initialize */
4902-
Oid element_type = get_element_type(array_type);
4903-
4904-
if (!OidIsValid(element_type))
4905-
ereport(ERROR,
4906-
(errcode(ERRCODE_DATATYPE_MISMATCH),
4907-
errmsg("data type %s is not an array type",
4908-
format_type_be(array_type))));
4909-
astate = initArrayResultArr(array_type, element_type, rcontext);
4910-
}
4944+
astate = initArrayResultArr(array_type, InvalidOid, rcontext, true);
49114945
else
4912-
{
49134946
Assert(astate->array_type == array_type);
4914-
}
49154947

49164948
oldcontext = MemoryContextSwitchTo(astate->mcontext);
49174949

@@ -5090,7 +5122,10 @@ makeArrayResultArr(ArrayBuildStateArr *astate,
50905122

50915123
/* Clean up all the junk */
50925124
if (release)
5125+
{
5126+
Assert(astate->private_cxt);
50935127
MemoryContextDelete(astate->mcontext);
5128+
}
50945129

50955130
return PointerGetDatum(result);
50965131
}
@@ -5106,9 +5141,10 @@ makeArrayResultArr(ArrayBuildStateArr *astate,
51065141
*
51075142
* input_type is the input datatype (either element or array type)
51085143
* rcontext is where to keep working state
5144+
* subcontext is a flag determining whether to use a separate memory context
51095145
*/
51105146
ArrayBuildStateAny *
5111-
initArrayResultAny(Oid input_type, MemoryContext rcontext)
5147+
initArrayResultAny(Oid input_type, MemoryContext rcontext, bool subcontext)
51125148
{
51135149
ArrayBuildStateAny *astate;
51145150
Oid element_type = get_element_type(input_type);
@@ -5118,7 +5154,7 @@ initArrayResultAny(Oid input_type, MemoryContext rcontext)
51185154
/* Array case */
51195155
ArrayBuildStateArr *arraystate;
51205156

5121-
arraystate = initArrayResultArr(input_type, element_type, rcontext);
5157+
arraystate = initArrayResultArr(input_type, InvalidOid, rcontext, subcontext);
51225158
astate = (ArrayBuildStateAny *)
51235159
MemoryContextAlloc(arraystate->mcontext,
51245160
sizeof(ArrayBuildStateAny));
@@ -5133,7 +5169,7 @@ initArrayResultAny(Oid input_type, MemoryContext rcontext)
51335169
/* Let's just check that we have a type that can be put into arrays */
51345170
Assert(OidIsValid(get_array_type(input_type)));
51355171

5136-
scalarstate = initArrayResult(input_type, rcontext);
5172+
scalarstate = initArrayResult(input_type, rcontext, subcontext);
51375173
astate = (ArrayBuildStateAny *)
51385174
MemoryContextAlloc(scalarstate->mcontext,
51395175
sizeof(ArrayBuildStateAny));
@@ -5159,7 +5195,7 @@ accumArrayResultAny(ArrayBuildStateAny *astate,
51595195
MemoryContext rcontext)
51605196
{
51615197
if (astate == NULL)
5162-
astate = initArrayResultAny(input_type, rcontext);
5198+
astate = initArrayResultAny(input_type, rcontext, true);
51635199

51645200
if (astate->scalarstate)
51655201
(void) accumArrayResult(astate->scalarstate,

‎src/backend/utils/adt/xml.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3948,7 +3948,7 @@ xpath(PG_FUNCTION_ARGS)
39483948
ArrayType *namespaces = PG_GETARG_ARRAYTYPE_P(2);
39493949
ArrayBuildState *astate;
39503950

3951-
astate = initArrayResult(XMLOID, CurrentMemoryContext);
3951+
astate = initArrayResult(XMLOID, CurrentMemoryContext, true);
39523952
xpath_internal(xpath_expr_text, data, namespaces,
39533953
NULL, astate);
39543954
PG_RETURN_ARRAYTYPE_P(makeArrayResult(astate, CurrentMemoryContext));

‎src/include/utils/array.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ typedef struct ArrayBuildState
8989
int16 typlen; /* needed info about datatype */
9090
bool typbyval;
9191
char typalign;
92+
bool private_cxt; /* use private memory context */
9293
} ArrayBuildState;
9394

9495
/*
@@ -109,6 +110,7 @@ typedef struct ArrayBuildStateArr
109110
int lbs[MAXDIM];
110111
Oid array_type; /* data type of the arrays */
111112
Oid element_type; /* data type of the array elements */
113+
bool private_cxt; /* use private memory context */
112114
} ArrayBuildStateArr;
113115

114116
/*
@@ -293,7 +295,7 @@ extern void deconstruct_array(ArrayType *array,
293295
extern bool array_contains_nulls(ArrayType *array);
294296

295297
extern ArrayBuildState *initArrayResult(Oid element_type,
296-
MemoryContext rcontext);
298+
MemoryContext rcontext, bool subcontext);
297299
extern ArrayBuildState *accumArrayResult(ArrayBuildState *astate,
298300
Datum dvalue, bool disnull,
299301
Oid element_type,
@@ -304,7 +306,7 @@ extern Datum makeMdArrayResult(ArrayBuildState *astate, int ndims,
304306
int *dims, int *lbs, MemoryContext rcontext, bool release);
305307

306308
extern ArrayBuildStateArr *initArrayResultArr(Oid array_type, Oid element_type,
307-
MemoryContext rcontext);
309+
MemoryContext rcontext, bool subcontext);
308310
extern ArrayBuildStateArr *accumArrayResultArr(ArrayBuildStateArr *astate,
309311
Datum dvalue, bool disnull,
310312
Oid array_type,
@@ -313,7 +315,7 @@ extern Datum makeArrayResultArr(ArrayBuildStateArr *astate,
313315
MemoryContext rcontext, bool release);
314316

315317
extern ArrayBuildStateAny *initArrayResultAny(Oid input_type,
316-
MemoryContext rcontext);
318+
MemoryContext rcontext, bool subcontext);
317319
extern ArrayBuildStateAny *accumArrayResultAny(ArrayBuildStateAny *astate,
318320
Datum dvalue, bool disnull,
319321
Oid input_type,

‎src/pl/plperl/plperl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1218,7 +1218,7 @@ plperl_array_to_datum(SV *src, Oid typid, int32 typmod)
12181218
errmsg("cannot convert Perl array to non-array type %s",
12191219
format_type_be(typid))));
12201220

1221-
astate = initArrayResult(elemtypid, CurrentMemoryContext);
1221+
astate = initArrayResult(elemtypid, CurrentMemoryContext, true);
12221222

12231223
_sv_to_datum_finfo(elemtypid, &finfo, &typioparam);
12241224

0 commit comments

Comments
 (0)