pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>
Cc: Tomas Vondra <tv@fuzzy.cz>
Cc: pgsql-hackers@postgresql.org
Subject: Re: SlabCheck leaks memory into TopMemoryContext
Date: Thu, 16 Jan 2020 21:00:38 +0100
Message-ID: <20200116200038.rfqfnqsrjcdv5kwk@development> (raw)
In-Reply-To: <2646.1579195983@sss.pgh.pa.us>
References: <20200116044119.g45f7pmgz4jmodxj@alap3.anarazel.de>
	<23952.1579151393@sss.pgh.pa.us>
	<20200116061732.vvpyexenaortpu5y@alap3.anarazel.de>
	<27213.1579155900@sss.pgh.pa.us>
	<20200116064143.opcu2vhcktddpzbh@alap3.anarazel.de>
	<19743.1579188421@sss.pgh.pa.us>
	<20200116162500.pcgxmykirm677mt6@development>
	<20200116164849.75rzho6zn4qppnf3@alap3.anarazel.de>
	<20200116170153.qp36chsyhikvgvxt@development>
	<2646.1579195983@sss.pgh.pa.us>

On Thu, Jan 16, 2020 at 12:33:03PM -0500, Tom Lane wrote:
>Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
>> On Thu, Jan 16, 2020 at 08:48:49AM -0800, Andres Freund wrote:
>>> I don't get why it's advantageous to allocate this once for each slab,
>>> rather than having it as a global once for all slabs. But anyway, still
>>> clearly better than the current situation.
>
>> It's largely a matter of personal preference - I agree there are cases
>> when global variables are the best solution, but I kinda dislike them.
>> It seems cleaner to just allocate it as part of the slab, not having to
>> deal with different number of chunks per block between slabs.
>> Plus we don't have all that many slabs (like 2), and it's only really
>> used in debug builds anyway. So I'm not all that woried about this
>> wasting a couple extra kB of memory.
>
>A positive argument for doing it like this is that the overhead goes away
>when the SlabContexts are all deallocated, while a global variable would
>presumably stick around indefinitely.  But I concur that in current usage,
>there's hardly any point in worrying about the relative benefits.  We
>should just keep it simple, and this seems marginally simpler than the
>other way.
>

I think the one possible argument against this approach might be that it
adds a field to the struct, so if you have an extension using a Slab
context, it'll break if you don't rebuild it. But that only matters if
we want to backpatch it (which I think is not the plan) and with memory
context checking enabled (which does not apply to regular packages).


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services 





view thread (20+ messages)  latest in thread

Message-ID: <20200116200038.rfqfnqsrjcdv5kwk@development>
Permalink:  ../20200116200038.rfqfnqsrjcdv5kwk@development/
Also on:    postgresql.org/message-id/20200116200038.rfqfnqsrjcdv5kwk@development

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tomas.vondra@2ndquadrant.com, tgl@sss.pgh.pa.us, andres@anarazel.de, tv@fuzzy.cz
  Subject: Re: SlabCheck leaks memory into TopMemoryContext
  In-Reply-To: <20200116200038.rfqfnqsrjcdv5kwk@development>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox