Re: XID formatting and SLRU refactorings (was: Add 64-bit XIDs into PostgreSQL 15)

On Mon, 6 Nov 2023 at 18:01, Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
>
> On Mon, Nov 6, 2023 at 3:42 PM Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> wrote:
> >
> > On Mon, 6 Nov 2023 at 17:07, Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
> > > On Wed, Jul 5, 2023 at 4:46 PM Aleksander Alekseev <aleksander(at)timescale(dot)com> wrote:
> > > > PFE the corrected patchset v58.
> > >
> > > I'd like to revive this thread.
> > >
> > > This patchset is extracted from a larger patchset implementing 64-bit xids. It converts page numbers in SLRUs into 64 bits. The most SLRUs save the same file naming scheme, thus their on-disk representation remains the same. However, the patch 0002 converts pg_notify to actually use a new naming scheme. Therefore pg_notify can benefit from simplification and getting rid of wraparound.
> > >
> > > -#define TransactionIdToCTsPage(xid) \
> > > - ((xid) / (TransactionId) COMMIT_TS_XACTS_PER_PAGE)
> > > +
> > > +/*
> > > + * Although we return an int64 the actual value can't currently exceeed 2**32.
> > > + */
> > > +static inline int64
> > > +TransactionIdToCTsPage(TransactionId xid)
> > > +{
> > > + return xid / (int64) COMMIT_TS_XACTS_PER_PAGE;
> > > +}
> > >
> > > Is there any reason we transform macro into a function? If not, I propose to leave this as a macro. BTW, there is a typo in a word "exceeed".
> > If I remember right, the compiler will make equivalent code from
> > inline functions and macros, and functions has an additional benefit:
> > the compiler will report type mismatch if any. That was the only
> > reason.
>
> Then it's OK to leave it as an inline function.
>
> > Also, I looked at v58-0001 and don't quite agree with mentioning the
> > authors of the original 64-xid patch, from which the current patch is
> > derived, as just "privious input" persons.
>
> +1, for converting all "previous input" persons as additional authors.
> That would be a pretty long list of authors though.
> BTW, I'm a bit puzzled on who should be the first author now?
Thanks! It's long, I agree, but the activity around this was huge and
involved many people, the patch itself already has more than
half-hundred iterations to date. I think at least people mentioned in
the commit message of v58 are fair to have author status.

As for the first, I'm not sure, it's hard for me to evaluate what is
more important, the initial prototype, or the final improvement
iterations. I don't think the existing order in a commit message has
some meaning. Maybe it's worth throwing a dice.

Regards, Pavel