On Mon, Dec 29, 2025 at 8:14 PM Álvaro Herrera <alvherre@kurilemu.de> wrote: > > On 2025-Dec-24, Ashutosh Bapat wrote: > > > If we go this route, we at least need to declare the new functions as > > static inline and move them to a header file instead of .c file. > > Hmm, why would we make them static inline instead of standard (extern) > functions? We use static inline functions when we want to avoid the > overhead of a function call in a hot code path, but I doubt that's the > case here. Am I mistaken on this? >
I wasn't aware that we are using static inline only in hot code paths. Looking around I see most of the static inline functions are from modules which are used in hot code paths. So, yeah that seems to be the convention. I also see some exceptions like those in basebackup_sink.h - I don't think all of those are used in hot code paths.
In this case, we are moving three assignments into their own functions. CPU instructions to call extern functions will be significant compared to CPU instructions for those assignments. static inline functions, OTOH, would have similar performance as the existing code while providing modularization. If you feel that's not a good enough reason, I am ok keeping them extern.
> > Further, does it make sense to put together all the state variables > > into a single structure? > > Yeah -- keeping the threaded-backend project in mind, moving them to a > single struct seems to make sense. I think it's a separate patch though > because it'd be more invasive than Chao's initial patch, as those > variables are used in many places. >
Attached v3 patch set. Comparing to v2, the changes are:
0001:
* Combine the two cleanup functions into one and control them by a bool flag.
* Change the helper function to be extern.
* Move out cleanup from reset function.
0002: Consolidate replication origin session globals into a single state struct.
