Re: Report a potential memory leak in setup_config()

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, wliang(at)stu(dot)xidian(dot)edu(dot)cn, pgsql-bugs <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Subject: Re: Report a potential memory leak in setup_config()
Date: 2022-02-16 17:53:47
Message-ID: 3388816.1645034027@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2022-02-15 23:30:07 -0500, Tom Lane wrote:
>> Hmm, I did it the other way, as attached.

> My goal when I did this was to improve the performance, rather than reduce the
> memory usage (this was a few months back). It's clearly better to perform all
> the replacements of a line soon after each other, rather than iterate over all
> the lines, once for each replacement. The latter is guaranteed to not have the
> data in L2/L1 anymore.

I'm skeptical that that effect is large enough to be interesting ...

> But if we move to not needing replacement for postgres.bki anymore, that's not
> an important goal anymore.

... and as you say, it won't matter if we push this processing to
the backend. So I'd prefer to go with the less-invasive change.

> Not that it matters anymore: At least for postgres.bki, it doesn't seem to
> make sense to use a line based approach in the first place? Seems like it
> should really be a processing the input file as a whole, doing all the
> replacements, into a resizable output buffer.

If we push this to the backend then it'd all have to be rethought.
I kind of like Peter's idea that maybe the run-time-variable items
could be handled by doing UPDATEs after the initial bootstrap.
(Very far down the road, maybe that would put us in a position where
the bootstrap phase could be replaced by copying some prefab data files.)

>> It seems that actually the pointer arrays *are* a big chunk of
>> the leakage, because the individual strings get freed in the
>> output loops!

> Right, isn't that what I had said?

I hadn't absorbed the existence of the per-string free() calls;
the fact that the reported leakage was close to the size of
postgres.bki misled me into thinking we were leaking the
strings too.

regards, tom lane

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Tomas Vondra 2022-02-17 00:16:47 Re: BUG #17406: Segmentation fault on GiST index after 14.2 upgrade
Previous Message Andres Freund 2022-02-16 17:37:18 Re: Report a potential memory leak in setup_config()