Hi John, On Thu, Jul 18, 2024 at 02:22:08PM -0700, John H wrote: > Hi Bertrand, > > > 1 === > > ... > > That's worth additional comments in the code. > > There's this comment already about caching the value already, not sure > if you prefer something more? > > /* Cache values to reduce contention on lock */
Yeah, at the same place as the static lsn[] declaration, something like: static XLogRecPtr lsn[NUM_SYNC_REP_WAIT_MODE]; /* cached LSNs */ but that may just be a matter of taste. > > 3 === > > ... > > NUM_SYNC_REP_WAIT_MODE is small but as the goal is the keep the lock time as > > short as possible I wonder if it wouldn't be better to use memcpy() here > > instead > > of this for loop. > > > > It results in a "Wdiscarded-qualifiers" which is safe given we take > the lock, but adds noise? > What do you think? > > "slot.c:2756:46: warning: passing argument 2 of ‘memcpy’ discards > ‘volatile’ qualifier from pointer target type > [-Wdiscarded-qualifiers]" Right, we may want to cast it then but given that the for loop is "small" I think that's also fine to keep the for loop. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
