Re: tweaking NTUP_PER_BUCKET - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: tweaking NTUP_PER_BUCKET
Date
Msg-id 53F48F11.3070600@vmware.com
Whole thread Raw
In response to Re: tweaking NTUP_PER_BUCKET  (Tomas Vondra <tv@fuzzy.cz>)
Responses Re: tweaking NTUP_PER_BUCKET
List pgsql-hackers
On 07/20/2014 07:17 PM, Tomas Vondra wrote:
> On 19.7.2014 20:24, Tomas Vondra wrote:
>> On 13.7.2014 21:32, Tomas Vondra wrote:
>>> The current patch only implemnents this for tuples in the main
>>> hash table, not for skew buckets. I plan to do that, but it will
>>> require separate chunks for each skew bucket (so we can remove it
>>> without messing with all of them). The chunks for skew buckets
>>> should be smaller (I'm thinking about ~4kB), because there'll be
>>> more of them, but OTOH those buckets are for frequent values so the
>>> chunks should not remain empty.
>>
>> I've looked into extending the dense allocation to the skew buckets,
>> and I think we shouldn't do that. I got about 50% of the changes and
>> then just threw it out because it turned out quite pointless.
>>
>> The amount of memory for skew buckets is limited to 2% of work mem,
>> so even with 100% overhead it'll use ~4% of work mem. So there's
>> pretty much nothing to gain here. So the additional complexity
>> introduced by the dense allocation seems pretty pointless.
>>
>> I'm not entirely happy with the code allocating some memory densely
>> and some using traditional palloc, but it certainly seems cleaner
>> than the code I had.
>>
>> So I think the patch is mostly OK as is.
>
> Attached is v4 of the patch, mostly with minor improvements and cleanups
> (removed MemoryContextStats, code related to skew buckets).

Can you remind me what kind of queries this is supposed to help with? 
Could you do some performance testing to demonstrate the effect? And 
also a worst-case scenario.

At a quick glance, I think you're missing a fairly obvious trick in 
ExecHashIncreaseNumBatches: if a chunk contains only a single tuple, you 
can avoid copying it to a new chunk and just link the old chunk to the 
new list instead. Not sure if ExecHashIncreaseNumBatches is called often 
enough for that to be noticeable, but at least it should help in extreme 
cases where you push around huge > 100MB tuples.

- Heikki



pgsql-hackers by date:

Previous
From: Jeevan Chalke
Date:
Subject: Re: add line number as prompt option to psql
Next
From: Greg Stark
Date:
Subject: Re: Hokey wrong versions of libpq in apt.postgresql.org