On Thu, Sep 25, 2025 at 04:11:14PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> LGTM. I considered suggesting initializing the delay before the loop and
>> then updating it at the end of the loop, but that moves the
>> CHECK_FOR_INTERRUPTS between the delay calculation and the WaitLatch(),
>> which seems like it might extend the sleeps a bit (although that might be
>> negligible in practice).
>
> Yeah, I did not consider changing the fundamental logic of the loop.
> It's true that this implementation will use a minimum of three
> GetCurrentTimestamp() calls where in principle you should need only
> two when WaitLatch() doesn't exit early. I'm having a hard time
> getting excited about that though. We know that on just about all
> current platforms, reading the clock is a handful-of-nanoseconds
> operation. (cf. nearby discussion of GNU/Hurd where we found that
> that OS is far behind the curve; but I imagine they'll have fixed
> that by the time anyone would consider Hurd ready for production.)
> So it seems pretty negligible in a function that currently has 1ms
> delay resolution and is unlikely to have better than 1us resolution
> in the future.
Agreed, I'm not too worried about the system calls in this case. I think I
was more interested in seeing whether we could avoid the complicated float
handling. Something else that seems to work is moving the initial endtime
calculation to within the loop, like so:
- delay = endtime - GetNowFloat();
+ if (first)
+ {
+ endtime = GetNowFloat() + secs;
+ delay = secs;
+ first = false;
+ }
+ else
+ delay = endtime - GetNowFloat();
But, in any case, your patch still LGTM.
--
nathan
