On Fri, Dec 01, 2023 at 02:00:54PM +0900, Michael Paquier wrote:
> - 0006 includes the backend changes, that caches a set of callback
> routines for each sequence Relation, with an optional rd_tableam.
> Callbacks are documented in sequenceam.h. Perhaps the sequence RMGR
> renames should be split into a patch of its own, or just let as-is as
> as it could be shared across more than one AM, but I did not see a
> huge argument one way or another. The diffs are not that bad
> considering that the original patch at +1200 lines for src/backend/,
> with less documentation for the internal callbacks:
> 45 files changed, 1414 insertions(+), 718 deletions(-)
While looking at the patch set, I have noticed that the previous patch
0006 for the backend changes could be split into two patches to make
the review much easier, as of
- A first patch moving the code related to the in-core sequence AM
from commands/sequence.c to access/sequence/local.c, reshaping the
sequence RMGR:
12 files changed, 793 insertions(+), 611 deletions(-)
- A second patch to introduce the callbacks, the relcache and the
backend pieces, renaming the contents moved to local.c by the first
patch switching it to the handler:
38 files changed, 661 insertions(+), 155 deletions(-)
So please find attached a v2 set, with some typos fixed on top of this
extra split.
While on it, I have been doing some performance tests to see the
effect of the extra function pointers from the handler, required for
the computation of nextval(), using:
- Postgres on a tmpfs, running on scissors.
- An unlogged sequence.
- "SELECT count(nextval('popo')) FROM generate_series(1,N);" where N >
0.
At N=5M, one of my perf machines takes 3230ms in average to run the
query on HEAD (646ns per value, 20 runs), and 3315ms with the patch
(663ns, 20 runs), which is.. Err, not noticeable. But perhaps
somebody has a better idea of tests, say more micro-benchmarking
around nextval_internal()?
--
Michael