Hi, On 2023-01-17 12:26:57 -0500, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > Here's an updated version of the move to representing instr_time as > > nanoseconds. It's now split into a few patches: > > I took a quick look through this.
Thanks! > > 0001) Add INSTR_TIME_SET_ZERO() calls where otherwise 0002 causes gcc to > > warn > > Alternatively we can decide to deprecate INSTR_TIME_SET_ZERO() and > > just allow to assign 0. > > I think it's probably wise to keep the macro. If we ever rethink this > again, we'll be glad we kept it. Similarly, IS_ZERO is a good idea > even if it would work with just compare-to-zero. Perhaps an INSTR_TIME_ZERO() that could be assigned in variable definitions could give us the best of both worlds? > I'm almost tempted to suggest you define instr_time as a struct with a > uint64 field, just to help keep us honest about that. I can see that making sense. Unless somebody pipes up with opposition to that plan soon, I'll see how it goes. > > 0003) Add INSTR_TIME_SET_SECOND() > > This is used in 0004. Just allows setting an instr_time to a time in > > seconds, allowing for a cheaper loop exit condition in 0004. > > Code and comments are inconsistent about whether it's SET_SECOND or > SET_SECONDS. I think I prefer the latter, but don't care that much. That's probably because I couldn't decide... So I'll go with your preference. > > 0004) report nanoseconds in pg_test_timing > > Didn't examine 0004 in any detail, but the others look good to go > other than these nits. Thanks for looking! Greetings, Andres Freund