Re: Proposal for fixing intra-query memory leaks

Karel Zak <zakkr(at)zf(dot)jcu(dot)cz> writes:
>> (Is there any value in allowing for other memory context types? We could

> I have "aset" variant, that allow create and work with memory context in
> a shared memory.

Hmm. Well, maybe nailing things down to a single kind of context is
pushing it too far. What I mainly want is to be able to free a palloc'd
chunk without assuming that it came from the currently active context.
With AllocSets as aset.c does them, this is possible because every chunk
has a back-link to its owning context. We could still make it work with
multiple context types if we require that they all have the same kind of
back-link in the same place. Essentially, the overhead data for an
allocated chunk would have to be the same for all context types. But
we could still have different context types with different allocation
methods for each type. pfree macro would get the back-link from 8 bytes
before the given chunk address, verify that it points at something with
the right node type to be a memory context header, and then call through
a function pointer in the context header to get to the routine that
actually knows how to free a chunk of that context type.

> A questuon, need we really any name for context? I mean an example
> "CreateGlobalMemory(char *name)". Global memory routines expect that
> a context name is static. It is a litle problem if contexts are created
> dynamic. I suggest change it and allocate context name in context memory.

Good point. I don't believe that contexts have names at all at the
moment (portals do, but not contexts). But it seems like it would be
helpful for debugging to have some kind of name for each context. It
wouldn't have to be unique or anything, but when you were trying to
figure out where your memory leak is, being able to see which context
has gotten bloated would be useful...

> About blocks deallocation, now if context is destroyed for all blocks are
> call free(). And if some context needs new block the aset.c call malloc().
> What create a "BlockFreeList" with limited size (and allow set size for
> this pool as new postmaster command line switch), and instead call the free()
> for each block, remove a block (standard block, not a large) to this
> BlockFreeList and instead the malloc() try first look at to this list?

I think this is going in the wrong direction. We are not trying to
write a better malloc than malloc. What we are trying to do is make
allocation of small chunks very fast and make it possible to release a
whole bunch of related chunks without keeping track of them individually.
I don't believe that we need to improve on malloc at the level of
grabbing or releasing whole blocks.

> IMHO is very easy implement pfree() and repalloc() independent on
> CurrentMemoryContext setting. The MemoryContextSwitchTo() needs palloc()
> only. Or not?

Yes, but *only if the chunk you are being asked to pfree came from an
AllocSet-style context*. Currently it could have come from a different
kind of context that isn't based on AllocSets. (I think we have some
contexts where palloc is just a direct malloc, for example.)

> And what you mean about total persistent contexts in shared memory?
> I create for the QueryCache specific top context that is stored in the
> QueryCache pool (shmem) and is independent on standard TopMemoryContext.

QueryCache would probably be a permanent context ...

regards, tom lane