Re: Macro customizable hashtable / bitmapscan & aggregation perf

From: Mark Dilger <hornschnorter(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Macro customizable hashtable / bitmapscan & aggregation perf
Date: 2016-12-09 23:21:36
Message-ID: 783695C3-B7E6-4BE5-AB2A-99677EDAB7D9@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres,

Your patch, below, appears to have been applied to master in commit
5dfc198146b49ce7ecc8a1fc9d5e171fb75f6ba5. It makes mention of a
function, tuplehash_start_iterate, in a macro, but the function is not
defined or declared, and its signature and intention is not clear. Is there
any chance you could add some documentation about how this function
is intended to be used and defined?

See InitTupleHashIterator in src/include/nodes/execnodes.h

Thanks,

Mark Dilger

> On Oct 9, 2016, at 4:38 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> Attached is an updated version of the patchset. The main changes are
> - address most of Tomas' feedback
> - address regression test output changes by adding explicit ORDER BYs,
> in a separate commit.
> - fix issues around hash tables sized up to PG_UINT32_MAX
> - fix a bug in iteration with "concurrent" deletions
>
>>> We didn't really for dynahash (as it basically assumed a fillfactor of 1
>>> all the time), not sure if changing it won't also cause issues.
>>>
>>
>> That's a case of glass half-full vs. half-empty, I guess. If we assumed load
>> factor 1.0, then I see it as accounting for load factor (while you see it as
>> not accounting of it).
>
> Well, that load factor is almost never achieved, because we'd have grown
> since... I added a comment about disregarding fill factor and growth
> policy to estimate_hashagg_tablesize, which is actually the place where
> it'd seemingly make sense to handle it.
>
> Tomas, did you have time to run a benchmark?
>
> I think this is starting to look good.
>
>
> Regards,
>
> Andres
> <0001-Add-likely-unlikely-branch-hint-macros.patch><0002-Make-regression-tests-less-dependent-on-hash-table-o.patch><0003-Add-a-macro-customizable-hashtable.patch><0004-Use-more-efficient-hashtable-for-tidbitmap.c-to-spee.patch><0005-Use-more-efficient-hashtable-for-execGrouping.c-to-s.patch>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2016-12-09 23:26:47 Re: Macro customizable hashtable / bitmapscan & aggregation perf
Previous Message Peter Geoghegan 2016-12-09 22:59:25 Re: tuplesort_gettuple_common() and *should_free argument