Re: BUG #16112: large, unexpected memory consumption - Mailing list pgsql-bugs
From | Tomas Vondra |
---|---|
Subject | Re: BUG #16112: large, unexpected memory consumption |
Date | |
Msg-id | 20200411183552.u64s3w4nzvroiqxj@development Whole thread Raw |
In response to | Re: BUG #16112: large, unexpected memory consumption (Ben Cornett <ben@lantern.is>) |
Responses |
Re: BUG #16112: large, unexpected memory consumption
|
List | pgsql-bugs |
On Thu, Apr 09, 2020 at 08:21:54AM -0400, Ben Cornett wrote: >Hi, > >Did this patch ever get applied? It is not clear to me that it did. > Hmmm, I don't think it got applied, which means we've missed yet another minor release :-( Andres, any plans to push this? regards >Thanks! > >On Thu, Nov 14, 2019 at 06:47:07PM -0800, Andres Freund wrote: >> Hi, >> >> On 2019-11-13 13:03:31 -0500, Tom Lane wrote: >> > Andres Freund <andres@anarazel.de> writes: >> > > Thinking about it for a second longer, I don't think we'd need a new >> > > context - afaict argcontext has exactly the lifetime requirements >> > > needed. >> > >> > Hm, maybe so. That'd definitely be a better solution if it works. >> >> Here's a patch doing so. I think it'd be a good idea to rename >> argcontext into something like setcontext or such. But as I'm not that >> happy with the name, I've not yet made that change. >> >> Greetings, >> >> Andres Freund > >> diff --git i/src/backend/executor/execSRF.c w/src/backend/executor/execSRF.c >> index c8a3efc3654..85f33054265 100644 >> --- i/src/backend/executor/execSRF.c >> +++ w/src/backend/executor/execSRF.c >> @@ -114,10 +114,23 @@ ExecMakeTableFunctionResult(SetExprState *setexpr, >> ReturnSetInfo rsinfo; >> HeapTupleData tmptup; >> MemoryContext callerContext; >> - MemoryContext oldcontext; >> bool first_time = true; >> >> - callerContext = CurrentMemoryContext; >> + /* >> + * Execute per-tablefunc actions in appropriate context. >> + * >> + * The FunctionCallInfo needs to live across all the calls to a >> + * ValuePerCall function, so it can't be allocated in the per-tuple >> + * context. Similarly, the function arguments need to be evaluated in a >> + * context that is longer lived than the per-tuple context: The argument >> + * values would otherwise disappear when we reset that context in the >> + * inner loop. As the caller's CurrentMemoryContext is typically a >> + * query-lifespan context, we don't want to leak memory there. We require >> + * the caller to pass a separate memory context that can be used for this, >> + * and can be reset each time through to avoid bloat. >> + */ >> + MemoryContextReset(argContext); >> + callerContext = MemoryContextSwitchTo(argContext); >> >> funcrettype = exprType((Node *) setexpr->expr); >> >> @@ -164,20 +177,9 @@ ExecMakeTableFunctionResult(SetExprState *setexpr, >> setexpr->fcinfo->fncollation, >> NULL, (Node *) &rsinfo); >> >> - /* >> - * Evaluate the function's argument list. >> - * >> - * We can't do this in the per-tuple context: the argument values >> - * would disappear when we reset that context in the inner loop. And >> - * the caller's CurrentMemoryContext is typically a query-lifespan >> - * context, so we don't want to leak memory there. We require the >> - * caller to pass a separate memory context that can be used for this, >> - * and can be reset each time through to avoid bloat. >> - */ >> - MemoryContextReset(argContext); >> - oldcontext = MemoryContextSwitchTo(argContext); >> + /* evaluate the function's argument list */ >> + Assert(CurrentMemoryContext == argContext); >> ExecEvalFuncArgs(fcinfo, setexpr->args, econtext); >> - MemoryContextSwitchTo(oldcontext); >> >> /* >> * If function is strict, and there are any NULL arguments, skip >> @@ -217,7 +219,7 @@ ExecMakeTableFunctionResult(SetExprState *setexpr, >> CHECK_FOR_INTERRUPTS(); >> >> /* >> - * reset per-tuple memory context before each call of the function or >> + * Reset per-tuple memory context before each call of the function or >> * expression. This cleans up any local memory the function may leak >> * when called. >> */ >> @@ -257,6 +259,8 @@ ExecMakeTableFunctionResult(SetExprState *setexpr, >> */ >> if (first_time) >> { >> + MemoryContext oldcontext; >> + >> oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory); >> tupstore = tuplestore_begin_heap(randomAccess, false, work_mem); >> rsinfo.setResult = tupstore; >> @@ -285,6 +289,8 @@ ExecMakeTableFunctionResult(SetExprState *setexpr, >> >> if (tupdesc == NULL) >> { >> + MemoryContext oldcontext; >> + >> /* >> * This is the first non-NULL result from the >> * function. Use the type info embedded in the >> @@ -378,15 +384,18 @@ no_function_result: >> */ >> if (rsinfo.setResult == NULL) >> { >> - MemoryContextSwitchTo(econtext->ecxt_per_query_memory); >> + MemoryContext oldcontext; >> + >> + oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory); >> tupstore = tuplestore_begin_heap(randomAccess, false, work_mem); >> rsinfo.setResult = tupstore; >> + MemoryContextSwitchTo(oldcontext); >> + >> if (!returnsSet) >> { >> int natts = expectedDesc->natts; >> bool *nullflags; >> >> - MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory); >> nullflags = (bool *) palloc(natts * sizeof(bool)); >> memset(nullflags, true, natts * sizeof(bool)); >> tuplestore_putvalues(tupstore, expectedDesc, NULL, nullflags); > > > -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
pgsql-bugs by date: