Re: A qsort template

On Wed, Jun 16, 2021 at 1:55 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
[v2 patch]

Hi Thomas,

I plan to do some performance testing with VACUUM, ANALYZE etc soon, to see
if I can detect any significant differences.

I did a quick check of the MacOS/clang binary size (no debug symbols):

master: 8108408
0001-0009: 8125224

Later, I'll drill down into the individual patches and see if anything
stands out.

There were already some comments for v2 upthread about formatting and an
overflow hazard, but I did find a few more things to ask about:

- For my curiosity, there are a lot of calls to qsort/qunique in the tree
-- without having looked exhaustively, do these patches focus on cases
where there are bespoke comparator functions and/or hot code paths?

- Aside from the qsort{_arg} precedence, is there a practical reason for
keeping the new global functions in their own files?

- 0002 / 0004

+/* Search and unique functions inline in header. */

The functions are pretty small, but is there some advantage for inlining
these?

- 0003

#include "lib/qunique.h" is not needed anymore.

This isn't quite relevant for the current patch perhaps, but I'm wondering
why we don't already call bsearch for RelationHasSysCache() and
RelationSupportsSysCache().

- 0008

+#define ST_COMPARE(a, b, cxt) \
+ DatumGetInt32(FunctionCall2Coll(&cxt->flinfo, cxt->collation, *a, *b))

This seems like a pretty heavyweight comparison, so I'm not sure inlining
buys us much, but it seems also there are fewer branches this way. I'll
come up with a test and see what happens.

--
John Naylor
EDB: http://www.enterprisedb.com