On 07/13/2016 07:37 PM, Tom Lane wrote:
Peter Geoghegan <p...@heroku.com> writes:
On Wed, Jul 13, 2016 at 7:53 AM, Tomas Vondra
<tomas.von...@2ndquadrant.com> wrote:
In the thread [1] dealing with hashjoin bug introduced in 9.5, Tom voiced
his dislike of dense_alloc. I kinda agree with him that introducing "local
allocators" may not be the best idea, and as dense_alloc was introduced by
me I was playing with the idea to wrap this into a regular memory context,
perhaps with some restrictions (e.g. no pfree). But I'm having trouble with
that approach ...

I think that the "no pfree()" restriction would be necessary to get
the same benefit. But, doesn't that undermine the whole idea of making
it a memory context?

The other thing that doesn't seem to square at all with a general-purpose
memory context is the desire to walk through the stored tuples directly,
knowing that they are adjacent.  That means nothing else can be allocated
via the same mechanism.  So I tend to agree that if we accept Tomas' three
requirements as non-negotiable, then trying to make the allocator match
the MemoryContext API is probably impractical.

My feeling at this point is that we should leave it alone until/unless
we see similar requirements elsewhere, and then look to see if we can
derive a common abstraction.  I always find that it's easier to design
APIs based on concrete use-cases than on guesses about what will be
needed.

I agree with both points.

I think the MemoryContext API may not be right abstraction for this. Given a hammer big enough it would probably work in the end, but it'd probably require changes to the public MemoryContext API (e.g. relaxing the StandardChunkHeader requirement). And that seems a bit too risky.

So we probably need a new independent abstraction for this, but doing that based on a single use case is a bit silly.


I wonder though if we don't already have another similar use-case in
the ad-hoc "slab allocators" in reorderbuffer.c.  We already know that
that code has performance issues, cf bug #14231, so I suspect there's
a redesign in its future anyway.


I'm not sure - I'm not familiar with reorderbuffer.c, but it seems to do a fair number of pfrees and such. Also, pfrees seem to be the root of the performance issue. I suspect the slab allocator (or rather the allocation strategy in general) may need rethinking, but let's discuss that in that thread.

regards

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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to