On Wed, Oct 26, 2022 at 03:05:20PM +0530, Bharath Rupireddy wrote: > A comment on the patch: > Isn't it better we track the soonest wakeup time or min of wakeup[] > array (update the min whenever the array element is updated in > WalRcvComputeNextWakeup()) instead of recomputing everytime by looping > for NUM_WALRCV_WAKEUPS times? I think this will save us some CPU > cycles, because the for loop, in which the below code is placed, runs > till the walreceiver end of life cycle. We may wrap wakeup[] and min > inside a structure for better code organization or just add min as > another static variable alongside wakeup[]. > > + /* Find the soonest wakeup time, to limit our nap. */ > + nextWakeup = PG_INT64_MAX; > + for (int i = 0; i < NUM_WALRCV_WAKEUPS; ++i) > + nextWakeup = Min(wakeup[i], nextWakeup); > + nap = Max(0, (nextWakeup - now + 999) / 1000);
While that might save a few CPU cycles when computing the nap time, I don't think it's worth the added complexity and CPU cycles in WalRcvComputeNextWakeup(). I suspect it'd be difficult to demonstrate any meaningful difference between the two approaches. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
