From: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Geoghegan <pg(at)heroku(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: rethinking dense_alloc (HashJoin) as a memory context |
Date: | 2016-07-13 18:10:59 |
Message-ID: | 50cdc043-a47f-8185-670a-72964ada8254@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 07/13/2016 07:37 PM, Tom Lane wrote:
> Peter Geoghegan <pg(at)heroku(dot)com> writes:
>> On Wed, Jul 13, 2016 at 7:53 AM, Tomas Vondra
>> <tomas(dot)vondra(at)2ndquadrant(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2016-07-13 18:44:15 | Re: Bug in batch tuplesort memory CLUSTER case (9.6 only) |
Previous Message | Corey Huinker | 2016-07-13 18:06:20 | Re: \timing interval |