On Wed, Jun 16, 2021 at 1:55 AM Thomas Munro <thomas.mu...@gmail.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
