Hi, Thanks for pushing the error handling cleanup etc!
On 2021-10-22 16:32:39 -0400, Tom Lane wrote: > I wrote: > > Andres Freund <and...@anarazel.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
