Re: Curing plpgsql's memory leaks for statement-lifespan values - Mailing list pgsql-hackers

Jim Nasby <Jim.Nasby@bluetreble.com> writes:
> On 7/25/16 1:50 PM, Tom Lane wrote:
>> There's a glibc-dependent hack in aset.c that reports any
>> plpgsql-driven palloc or pfree against a context named "SPI Proc", as
>> well as changes in pl_comp.c so that transient junk created during initial
>> parsing of a plpgsql function body doesn't end up in the SPI Proc context.
>> (I did that just to cut the amount of noise I had to chase down.  I suppose
>> in large functions it might be worth adopting such a change so that that
>> junk can be released immediately after parsing; but I suspect for most
>> functions it'd just be an extra context without much gain.)

> Some folks do create very large plpgsql functions, so if there's a handy
> way to estimate the size of the function (pg_proc.prosrc's varlena size
> perhaps) then it might be worth doing that for quite large functions.

I poked at that a bit and realized that during a normal plpgsql function
call, there isn't anything in the SPI Proc context when do_compile is
entered, which means that we could flush all the transient cruft by just
resetting the context afterwards.  The sanest place to put that seems to
be in plpgsql_call_handler, as per attached.  We could put it in
plpgsql_compile so it's not hit in the main line code path, but that would
require plpgsql_compile to know that it's called in an empty context,
which seems a bit fragile (and indeed probably false in the forValidator
case).

There isn't any equivalently easy way to clean up in the case of a DO
block, but I'm not sure that we care as much in that case.

I'm not sure that this is really a net win.  MemoryContextReset is pretty
cheap (at least in non-cassert builds) but it's not zero cost, and in
most scenarios we're not going to care that much about the extra space.
It appeared to me after collecting some stats about the functions present
in the regression tests that for larger functions, the extra space eaten
is just some small multiple (like 2x-3x) of the function body string
length.  So it's not *that* much data, even for big functions, and it's
only going to be eaten on the first usage of a function within a session.

            regards, tom lane

diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c
index 36868fb..2a36913 100644
*** a/src/pl/plpgsql/src/pl_handler.c
--- b/src/pl/plpgsql/src/pl_handler.c
***************
*** 23,28 ****
--- 23,29 ----
  #include "utils/builtins.h"
  #include "utils/guc.h"
  #include "utils/lsyscache.h"
+ #include "utils/memutils.h"
  #include "utils/syscache.h"


*************** plpgsql_call_handler(PG_FUNCTION_ARGS)
*** 230,235 ****
--- 231,239 ----
      /* Find or compile the function */
      func = plpgsql_compile(fcinfo, false);

+     /* Flush any transient junk allocated during compile */
+     MemoryContextReset(CurrentMemoryContext);
+
      /* Must save and restore prior value of cur_estate */
      save_cur_estate = func->cur_estate;