Re: Suppressing useless wakeups in walreceiver

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