From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: Daniel Gustafsson <daniel@yesql.se>, wliang@stu.xidian.edu.cn,
        pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Subject: Re: Report a potential memory leak in setup_config()
In-reply-to: <20220216173718.yzav6udoxo3gavii@alap3.anarazel.de>
References: <64ee7543.20d.17efb1270ca.Coremail.wliang@stu.xidian.edu.cn>
 <A7070803-5A15-486E-AE04-ECC13CD10038@yesql.se>
 <3052163.1644942806@sss.pgh.pa.us>
 <20220216012953.6d7bzmsblqou3ru4@alap3.anarazel.de>
 <3239746.1644975946@sss.pgh.pa.us>
 <20220216022118.vvijtgd5rl2b6ucv@alap3.anarazel.de>
 <3243399.1644978358@sss.pgh.pa.us> <3244453.1644979091@sss.pgh.pa.us>
 <20220216041049.uy3vwtk6ry66g6um@alap3.anarazel.de>
 <3255493.1644985807@sss.pgh.pa.us>
 <20220216173718.yzav6udoxo3gavii@alap3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Wed, 16 Feb 2022 09:37:18 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3388815.1645034027.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Wed, 16 Feb 2022 12:53:47 -0500
Message-ID: <3388816.1645034027@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3388816.1645034027%40sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.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 redu=
ce the
> memory usage (this was a few months back). It's clearly better to perfor=
m all
> the replacements of a line soon after each other, rather than iterate ov=
er all
> the lines, once for each replacement. The latter is guaranteed to not ha=
ve 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 i=
t
> 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