Re: Experimenting with hash tables inside pg_dump

Hi,

Thanks for pushing the error handling cleanup etc!

On 2021-10-22 16:32:39 -0400, Tom Lane wrote:
> I wrote:
> > Andres Freund <andres(at)anarazel(dot)de> writes:
> >> 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.
>
> > Seems plausible --- you want me to go change that?
>
> Hmm, harder than it sounds. If I remove "inline" from SH_SCOPE then
> the compiler complains about unreferenced static functions, while
> if I leave it there than adding pg_noinline causes a complaint about
> conflicting options.

The easy way out would be to to not declare SH_GROW inside SH_DECLARE - that'd
currently work, because there aren't any calls to grow from outside of
simplehash.h. The comment says:
* ... But resizing to the exact input size can be advantageous
* performance-wise, when known at some point.

But perhaps that's sufficiently served to create the table with the correct
size immediately?

If we were to go for that, we'd just put SH_GROW in the SH_DEFINE section not
use SH_SCOPE, but just static. That works here, and I have some hope it'd not
cause warnings on other compilers either, because there'll be references from
the other inline functions. Even if there's a SH_SCOPE=static inline
simplehash use inside a header and there aren't any callers in a TU, there'd
still be static inline references to it.

Another alternative would be to use __attribute__((unused)) or such on
non-static-inline functions that might or might not be used.

> Seems like we need a less quick-and-dirty approach to dealing with
> unnecessary simplehash support functions.

I don't think the problem is unnecessary ones? It's "cold" functions we don't
want to have inlined into larger functions.

Greetings,

Andres Freund