Hi, On 2023-01-19 11:47:49 +0100, David Geier wrote: > > I also couldn't help and hacked a bit on the rdtsc pieces. I did figure out > > how to do the cycles->nanosecond conversion with integer shift and multiply > > in > > the common case, which does show a noticable speedup. But that's for another > > day. > I also have code for that here. I decided against integrating it because we > don't convert frequently enough to make it matter. Or am I missing > something?
We do currently do the conversion quite frequently. Admittedly I was partially motivated by trying to get the per-loop overhead in pg_test_timing down ;) But I think it's a real issue. Places where we do, but shouldn't, convert: - ExecReScan() - quite painful, we can end up with a lot of those - InstrStopNode() - adds a good bit of overhead to simple - PendingWalStats.wal_write_time - this is particularly bad because it happens within very contended code - calls to pgstat_count_buffer_read_time(), pgstat_count_buffer_write_time() - they can be very frequent - pgbench.c, as we already discussed - pg_stat_statements.c - ... These all will get a bit slower when moving to a "variable" frequency. What was your approach for avoiding the costly operation? I ended up with a integer multiplication + shift approximation for the floating point multiplication (which in turn uses the inverse of the division by the frequency). To allow for sufficient precision while also avoiding overflows, I had to make that branch conditional, with a slow path for large numbers of nanoseconds. > > I fought a bit with myself about whether to send those patches in this > > thread, > > because it'll take over the CF entry. But decided that it's ok, given that > > David's patches should be rebased over these anyway? > That's alright. > Though, I would hope we attempt to bring your patch set as well as the RDTSC > patch set in. I think it'd be great - but I'm not sure we're there yet, reliability and code-complexity wise. I think it might be worth makign the rdts aspect somewhat measurable. E.g. allowing pg_test_timing to use both at the same time, and have it compare elapsed time with both sources of counters. Greetings, Andres Freund
