On Thu, Jan 16, 2020 at 12:33:03PM -0500, Tom Lane wrote:
Tomas Vondra <tomas.von...@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
