Re: Experimenting with hash tables inside pg_dump

Hi,

On October 22, 2021 10:32:30 AM PDT, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>Andres Freund <andres(at)anarazel(dot)de> writes:
>>> On 2021-10-21 18:27:25 -0400, Tom Lane wrote:
>>>> (a) the executable size increases by a few KB --- apparently, even
>>>> the minimum subset of simplehash.h's functionality is code-wasteful.
>
>> If I prevent the compiler from inlining findObjectByCatalogId() in all the
>> find*ByOid() routines, your version is smaller than master even without other
>> changes.
>
>Hmm ... seems to depend a lot on which compiler you use.

Inline heuristics change a lot over time, so that'd make sense.

I see some win by marking pg_log_error cold. That might be useful more generally too.

Which made me look at the code invoking it from simplehash. I think the patch that made simplehash work in frontend code isn't quite right, because pg_log_error() returns...

Wonder if we should mark simplehash's grow as noinline? Even with a single caller it seems better to not inline it to remove register allocator pressure.

>Anyway, these are all sub-one-percent changes in the code
>size, so probably we should not sweat that much about it.
>I'm kind of leaning now towards pushing the patch, just
>on the grounds that getting rid of all those single-purpose
>index arrays (and likely future need for more of them)
>is worth it from a maintenance perspective.

+1

The only thought I had wrt the patch is that I'd always create the hash table. That way the related branches can be removed, which is a win code size wise (as well as speed presumably, but I think we're far away from that mattering).

This type of code is where I most wish for a language with proper generic data types/containers...

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.