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
