Re: SlabCheck leaks memory into TopMemoryContext

On Thu, Jan 16, 2020 at 08:48:49AM -0800, Andres Freund wrote:
>Hi,
>
>On 2020-01-16 17:25:00 +0100, Tomas Vondra wrote:
>> On Thu, Jan 16, 2020 at 10:27:01AM -0500, Tom Lane wrote:
>> > Andres Freund <andres(at)anarazel(dot)de> writes:
>> > > ... I thought you were asking whether
>> > > any additional memory could just be avoided...
>> >
>> > Well, I was kind of wondering that, but if it's not practical then
>> > preallocating the space instead will do.
>> >
>>
>> I don't think it's practical to rework the checks in a way that would
>> not require allocations. Maybe it's possible, but I think it's not worth
>> the extra complexity.
>>
>> The attached fix should do the trick - it pre-allocates the space when
>> creating the context. There is a bit of complexity because we want to
>> allocate the space as part of the context header, but nothin too bad. We
>> might optimize it a bit by using a regular bitmap (instead of just an
>> array of bools), but I haven't done that.
>
>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.

YMMV of course ...

regards

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