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
