Re: SlabCheck leaks memory into TopMemoryContext

2020-01-17 Thread Tomas Vondra
On Thu, Jan 16, 2020 at 01:41:39PM -0500, Tom Lane wrote: Andres Freund writes: On 2020-01-16 18:01:53 +0100, Tomas Vondra wrote: 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. IDK, making memory usage look differ

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-17 Thread Tomas Vondra
On Thu, Jan 16, 2020 at 06:04:32PM +0100, Tomas Vondra wrote: On Thu, Jan 16, 2020 at 11:43:34AM -0500, Tom Lane wrote: Tomas Vondra writes: 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

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tomas Vondra
On Thu, Jan 16, 2020 at 03:15:41PM -0500, Tom Lane wrote: Tomas Vondra writes: 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

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tom Lane
Tomas Vondra writes: > 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)

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tomas Vondra
On Thu, Jan 16, 2020 at 12:33:03PM -0500, Tom Lane wrote: Tomas Vondra 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 b

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tom Lane
Andres Freund writes: > On 2020-01-16 18:01:53 +0100, Tomas Vondra wrote: >> 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. > IDK, making memory usage look different makes optimizing it harder. Not > a hard rule, obv

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Andres Freund
Hi, On 2020-01-16 18:01:53 +0100, Tomas Vondra wrote: > Plus we don't have all that many slabs (like 2) FWIW, I have a local patch that adds additional ones, for the relcache and catcache, that's how I noticed the leak. Because a test pgbench absolutely *tanked* in performance. Just for giggles.

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tom Lane
Tomas Vondra 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

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tomas Vondra
On Thu, Jan 16, 2020 at 11:43:34AM -0500, Tom Lane wrote: Tomas Vondra writes: 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 mi

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tomas Vondra
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 writes: > > ... I thought you were asking whether > > any additional memory could just be avoided... >

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Andres Freund
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 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

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tom Lane
Tomas Vondra writes: > 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 (inst

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tomas Vondra
On Thu, Jan 16, 2020 at 10:27:01AM -0500, Tom Lane wrote: Andres Freund 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'

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tom Lane
Andres Freund 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. regards, tom lane

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-16 Thread Tomas Vondra
On Wed, Jan 15, 2020 at 10:41:43PM -0800, Andres Freund wrote: Hi, On 2020-01-16 01:25:00 -0500, Tom Lane wrote: Andres Freund writes: > On 2020-01-16 00:09:53 -0500, Tom Lane wrote: >> It's basically assuming that the memory management mechanism is sane, >> which makes the whole thing fundame

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-15 Thread Andres Freund
Hi, On 2020-01-16 01:25:00 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2020-01-16 00:09:53 -0500, Tom Lane wrote: > >> It's basically assuming that the memory management mechanism is sane, > >> which makes the whole thing fundamentally circular, even if it's > >> relying on some other c

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-15 Thread Tom Lane
Andres Freund writes: > On 2020-01-16 00:09:53 -0500, Tom Lane wrote: >> It's basically assuming that the memory management mechanism is sane, >> which makes the whole thing fundamentally circular, even if it's >> relying on some other context to be sane. Is there a way to do the >> checking with

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-15 Thread Andres Freund
Hi, On 2020-01-16 00:09:53 -0500, Tom Lane wrote: > Andres Freund writes: > > I just noticed that having a slab context around in an assertion build > > leads to performance degrading and memory usage going up. A bit of > > poking revealed that SlabCheck() doesn't free the freechunks it > > alloc

Re: SlabCheck leaks memory into TopMemoryContext

2020-01-15 Thread Tom Lane
Andres Freund writes: > I just noticed that having a slab context around in an assertion build > leads to performance degrading and memory usage going up. A bit of > poking revealed that SlabCheck() doesn't free the freechunks it > allocates. > It's on its own obviously trivial to fix. It seems

SlabCheck leaks memory into TopMemoryContext

2020-01-15 Thread Andres Freund
Hi Tomas, I just noticed that having a slab context around in an assertion build leads to performance degrading and memory usage going up. A bit of poking revealed that SlabCheck() doesn't free the freechunks it allocates. It's on its own obviously trivial to fix. But there's also this note at t