On Mon, 2022-03-28 at 11:00 -0700, Andres Freund wrote:
> I think this has been discussed before, but to me it's not obvious
> that it's a
> good idea to change RmgrTable from RmgrData to RmgrData *. That adds
> an
> indirection, without obvious benefit.
I did some performance tests. I created a narrow table, took a base
backup, loaded 100M records, finished the base backup. Then I recovered
using the different build combinations (patched/unpatched, clang/gcc).
compiler run1 run2
unpatched: gcc 102s 106s
patched: gcc 107s 105s
unpatched: clang 109s 110s
patched: clang 109s 111s
I don't see a clear signal that this patch worsens performance. The
102s run was the very first run, so I suspect it was just due to the
processor starting out cold. Let me know if you think the test is
testing the right paths.
Perhaps I should address your other perf concerns around GetRmgr (make
it static inline, reduce number of calls), and then leave the
indirection for the sake of cleanliness?
If you are still concerned, I can switch back to separate tables to
eliminate the indirection for built-in rmgrs. Separate rmgr tables
still require a branch (to decide which table to access), but it should
be a highly predictable one.
Regards,
Jeff Davis