Re: Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS
Hi, On 2023-02-13 18:14:58 -0800, Andrey Borodin wrote: > On Mon, Feb 13, 2023 at 4:39 PM Andres Freund wrote: > > > > The problem I'm talking about is the increased overhead in InstrStopNode(), > > due to BufferUsageAccumDiff() getting more expensive. > > > > Thanks, now I understand the problem better. According to godbolt.com > my patch doubles the number of instructions in this function. Unless > we compute only tables\indexes\matviews. > Anyway, without regarding functionality of this particular patch, > BufferUsageAccumDiff() does not seem slow to me. It's just a > branchless bunch of simd instructions. Performance of this function > might matter only when called gazillion times per second. It is called gazillions of times per second when you do an EXPLAIN (ANALYZE, BUFFERS). Every invocation of an executor node calls it. Here's a quick pgbench, showing todays code, with -O3, without assertions: 298.396 0 SELECT generate_series(1, 1000) OFFSET 1000; 397.400 0 EXPLAIN (ANALYZE, TIMING OFF) SELECT generate_series(1, 1000) OFFSET 1000; 717.238 0 EXPLAIN (ANALYZE, TIMING ON) SELECT generate_series(1, 1000) OFFSET 1000; 419.736 0 EXPLAIN (ANALYZE, BUFFERS, TIMING OFF) SELECT generate_series(1, 1000) OFFSET 1000; 761.575 0 EXPLAIN (ANALYZE, BUFFERS, TIMING ON) SELECT generate_series(1, 1000) OFFSET 1000; The effect ends up a lot larger once you add in joins etc, because it ads additional executor node that all have their instrumentation started/stopped. Greetings, Andres Freund
Re: Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS
On Mon, Feb 13, 2023 at 4:39 PM Andres Freund wrote: > > The problem I'm talking about is the increased overhead in InstrStopNode(), > due to BufferUsageAccumDiff() getting more expensive. > Thanks, now I understand the problem better. According to godbolt.com my patch doubles the number of instructions in this function. Unless we compute only tables\indexes\matviews. Anyway, without regarding functionality of this particular patch, BufferUsageAccumDiff() does not seem slow to me. It's just a branchless bunch of simd instructions. Performance of this function might matter only when called gazillion times per second. Best regards, Andrey Borodin. for godbolt.cpp Description: Binary data
Re: Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS
Hi, On 2023-02-13 16:36:25 -0800, Andrey Borodin wrote: > On Mon, Feb 13, 2023 at 4:29 PM Andres Freund wrote: > > > 1. Some more increments on hot paths. We have to add this tiny toll to > > > every single buffer hit, but it will be seldom of any use. > > > > Additionally, I bet it slows down EXPLAIN (ANALYZE, BUFFERS) noticeably. > > It's > > already quite expensive... > > > > I think collection of instrumentation is done unconditionally. > We always do that > pgBufferUsage.shared_blks_hit++; > when the buffer is in shared_buffers. The problem I'm talking about is the increased overhead in InstrStopNode(), due to BufferUsageAccumDiff() getting more expensive. Andres
Re: Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS
On Mon, Feb 13, 2023 at 4:29 PM Andres Freund wrote: > > 1. Some more increments on hot paths. We have to add this tiny toll to > > every single buffer hit, but it will be seldom of any use. > > Additionally, I bet it slows down EXPLAIN (ANALYZE, BUFFERS) noticeably. It's > already quite expensive... > I think collection of instrumentation is done unconditionally. We always do that pgBufferUsage.shared_blks_hit++; when the buffer is in shared_buffers. Best regards, Andrey Borodin.
Re: Buffer usage detailed by RelKind in EXPLAIN ANALYZE BUFFERS
Hi, On 2023-02-13 16:23:30 -0800, Andrey Borodin wrote: > But there are some caveats: > 1. Some more increments on hot paths. We have to add this tiny toll to > every single buffer hit, but it will be seldom of any use. Additionally, I bet it slows down EXPLAIN (ANALYZE, BUFFERS) noticeably. It's already quite expensive... > All in all I do not have an opinion if this feature is a good tradeoff. > What do you think? Does the feature look useful? Do we want a more > polished implementation? Unless the above issues could be avoided, I don't think so. Greetings, Andres Freund