Hi, On 2023-01-18 14:02:48 +0100, David Geier wrote: > On 1/16/23 18:37, Andres Freund wrote: > > I'm doubtful this is worth the complexity it incurs. By the time we convert > > out of the instr_time format, the times shouldn't be small enough that the > > accuracy is affected much. > > I don't feel strong about it and you have a point that we most likely only > convert ones we've accumulated a fair amount of cycles.
I think we can avoid the issue another way. The inaccuracy comes from the cycles_to_sec ending up very small, right? Right now your patch has (and probably my old version similarly had): cycles_to_sec = 1.0 / (tsc_freq * 1000); I think it's better if we have one multiplier to convert cycles to nanoseconds - that'll be a double comparatively close to 1. We can use that to implement INSTR_TIME_GET_NANOSECONDS(). The conversion to microseconds then is just a division by 1000 (which most compilers convert into a multiplication/shift combo), and the conversions to milliseconds and seconds will be similar. Because we'll never "wrongly" go into the "huge number" or "very small number" ranges, that should provide sufficient precision? We'll of course still end up with a very small number when converting a few nanoseconds to seconds, but that's ok because it's the precision being asked for, instead of loosing precision in some intermediate representation. > > Looking around, most of the existing uses of INSTR_TIME_GET_MICROSEC() > > actually accumulate themselves, and should instead keep things in the > > instr_time format and convert later. We'd win more accuracy / speed that > > way. > > > > I don't think the introduction of pg_time_usec_t was a great idea, but oh > > well. > Fully agreed. Why not replacing pg_time_usec_t with instr_time in a separate > patch? pgbench used to use instr_time, but it was replaced by somebody thinking the API is too cumbersome. Which I can't quite deny, even though I think the specific change isn't great. But yes, this should definitely be a separate patch. Greetings, Andres Freund