From: Tom Lane <tgl@sss.pgh.pa.us>
To: Heikki Linnakangas <hlinnaka@iki.fi>
cc: Robert Haas <robertmhaas@gmail.com>,
        Alvaro Herrera <alvherre@2ndquadrant.com>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: hyrax vs. RelationBuildPartitionDesc
In-reply-to: <2efba7eb-7fab-59db-68bd-9a03e39588eb@iki.fi>
References: 
 <CA+TgmoY3bRmGB6-DUnoVy5fJoreiBJ43rwMrQRCdPXuKt4Ykaw@mail.gmail.com>
 <20190313164232.GA5270@alvherre.pgsql>
 <CA+Tgmoabw8KW9rx6ThnUTR_ofJC8GwVaC+XMe7HkZvsG7J1rvA@mail.gmail.com>
 <7448.1552497345@sss.pgh.pa.us> <7961.1552498252@sss.pgh.pa.us>
 <CA+TgmoaZ021fRZawzU_gozwgHMo-tXJc12uf3MJoA6k=HZsLJA@mail.gmail.com>
 <10788.1552501588@sss.pgh.pa.us>
 <CA+TgmoYd3vqd26RiT4sp7Mez3oQf-AxAzdBpEeTyCNF-7OchhA@mail.gmail.com>
 <CA+TgmoY71GwY8WhQKBH+7yexyZEw6WDbFBtj2E3EDszc0LnSqQ@mail.gmail.com>
 <3969.1552581380@sss.pgh.pa.us>
 <CA+TgmoYg2NY3KTUFiuaCxDkW8iXD3o3H3wdXs6m-o-dGiYHD_Q@mail.gmail.com>
 <15943.1552601288@sss.pgh.pa.us>
 <2efba7eb-7fab-59db-68bd-9a03e39588eb@iki.fi>
Comments: In-reply-to Heikki Linnakangas <hlinnaka@iki.fi>
	message dated "Fri, 15 Mar 2019 09:32:22 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <30379.1552657187.1@sss.pgh.pa.us>
Date: Fri, 15 Mar 2019 09:39:47 -0400
Message-ID: <30380.1552657187@sss.pgh.pa.us>
Precedence: bulk

Heikki Linnakangas <hlinnaka@iki.fi> writes:
> On 15/03/2019 00:08, Tom Lane wrote:
>> Possibly it'd make sense to conditionally compile this so that we only
>> do it in a CLOBBER_CACHE_ALWAYS build.  I'm not very sure about that,
>> but arguably in a normal build the overhead of making and destroying
>> a context would outweigh the cost of the leaked memory.  The main
>> argument I can think of for doing it all the time is that having memory
>> allocation work noticeably differently in CCA builds than normal ones
>> seems like a recipe for masking normal-mode bugs from the CCA animals.

> Having CLOBBER_CACHE_ALWAYS behave differently sounds horrible.

I'm not entirely convinced of that.  It's already pretty different...

> We maintain a free list of AllocSetContexts nowadays, so creating a 
> short-lived context should be pretty cheap. Or if it's still too 
> expensive, we could create one short-lived context as a child of 
> TopMemoryContext, and reuse that on every call, resetting it at the end 
> of the function.

RelationBuildDesc potentially recurses, so a single context won't do.
Perhaps you're right that the context freelist would make this cheap
enough to not matter, but I'm not sure of that either.

What I'm inclined to do to get the buildfarm pressure off is to set
things up so that by default we do this only in CCA builds, but there's
a #define you can set from the compile command line to override that
decision in either direction.  Then, if somebody wants to investigate
the performance effects in more detail, they could twiddle it easily.
Depending on the results, we could alter the default policy.

			regards, tom lane