Re: PATCH: two slab-like memory allocators

On 10/04/2016 09:44 PM, John Gorman wrote:
> SlabContext has a parent context. It can delegate requests that
> it cannot handle to the parent context which is GenSlab. Genslab
> can send them to the sister aset context.
>

But Slab may also be used separately, not just as part of GenSlab
(actually, reorderbuffer has two such contexts). That complicates things
quite a bit, and it also seems a bit awkward, because:

(a) It'd require a flag in SlabContext (or perhaps a pointer to the
second context), which introduces coupling between the contexts.

(b) SlabContext was meant to be extremely simple (based on the "single
chunk size" idea), and this contradicts that a bit.

(c) It'd move chunks between the memory contexts in unpredictable ways
(although the user should treat it as a single context, and not reset
the parts independently for example).

>
> This could handle all reallocs so there will be no surprises.
>

Yeah, but it's also

>
> Remind me again why we cannot realloc in place for sizes
> smaller than chunkSize? GenSlab is happy to allocate smaller
> sizes and put them into the fixed size chunks.
>
> Maybe SlabAlloc can be happy with sizes up to chunkSize.
>
> if (size <= set->chunkSize)
> return MemoryContextAlloc(set->slab, size);
> else
> return MemoryContextAlloc(set->aset, size);
>

That'd be certainly possible, but it seems a bit strange as the whole
Slab is based on the idea that all chunks have the same size. Moreover,
people usually realloc() to a larger chunk, so it does not really fix
anything in practice.

In GenSlab, the situation is more complicated. But I don't like the
coupling / moving chunks between contexts, etc.

If realloc() support is a hard requirement, it immediately rules out
SlabContext() as an independent memory context. Which seems stupid, as
independent Slab contexts are quite useful for reorderbuffer use case.

For GenSlab the situation is less clear, as there probably are ways to
make it work, but I'd vote to keep it simple for now, and simply do
elog(ERROR) in the realloc() methods - both for Slab and GenSlab. The
current use case (reorderbuffer) does not need that, and it seems like a
can of worms to me.

regards

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