On 14/06/2024 16:56, Maxim Orlov wrote:
Hi!
While working on a making multix xact offsets 64-bit [0] I've
discovered a minor issue. The thing is that type 'xid' is used in
all macro, but it doesn't correct. Appropriate MultiXactId or
MultiXactOffset should be used, actually.
And the second thing, as Heikki Linnakangas points out, args naming
is also misleading.
Thanks!
Looks good to me at a quick glance. I'll try to review and commit these
properly by Monday.
Additionally, I made an optional patch 0003 to switch from macro to
inline functions. For me, personally, use of macro functions is
justified if we are dealing with different argument types, to make
polymorphic call. Which is not the case here. So, we can have more
control over types and still generate the same code in terms of
speed. See https://godbolt.org/z/KM8voadhs Starting from O1 function
is inlined, thus no overhead is noticeable. Anyway, it's up to the
actual commiter to decide does it worth it or not. Again, this
particular patch 0003 is completely optional.
I agree static inline functions are generally easier to work with than
macros. These particular macros were not too bad, though.
I'll bite the bullet and commit this one too unless someone objects.
It's late in the v17 release cycle, but these are local to multixact.c
so there's no risk of breaking extensions, and it seems good to do it
now since we're modifying the macros anyway.
--
Heikki Linnakangas
Neon (https://neon.tech)
