Re: Gather performance analysis

On 9/8/21 8:05 AM, Dilip Kumar wrote:
> On Tue, Sep 7, 2021 at 8:41 PM Tomas Vondra 
> <tomas.vondra@enterprisedb.com <mailto:tomas.vondra@enterprisedb.com>> 
> wrote:
> 
>     Hi,
> 
>     The numbers presented in this thread seem very promising - clearly
>     there's significant potential for improvements. I'll run similar
>     benchmarks too, to get a better understanding of this.
> 
> 
> Thanks for showing interest.
> 
> 
>     Can you share some basic details about the hardware you used?
>     Particularly the CPU model - I guess this might explain some of the
>     results, e.g. if CPU caches are ~1MB, that'd explain why setting
>     tup_queue_size to 1MB improves things, but 4MB is a bit slower.
>     Similarly, number of cores might explain why 4 workers perform better
>     than 8 or 16 workers.
> 
> 
> I have attached the output of the lscpu.  I think batching the data 
> before updating in the shared memory will win because we are avoiding 
> the frequent cache misses and IMHO the benefit will be more in the 
> machine with more CPU sockets.
> 
>     Now, this is mostly expected, but the consequence is that maybe things
>     like queue size should be tunable/dynamic, not hard-coded?
> 
> 
> Actually, my intention behind the tuple queue size was to just see the 
> behavior. Do we really have the problem of workers stalling on queue 
> while sending the tuple, the perf report showed some load on WaitLatch 
> on the worker side so I did this experiment.  I saw some benefits but it 
> was not really huge.  I am not sure whether we want to just increase the 
> tuple queue size or make it tunable,  but if we want to support 
> redistribute operators in future sometime then maybe we should make it 
> dynamically growing at runtime, maybe using dsa or dsa + shared files.
> 

Thanks. I ran a couple more benchmarks, with different queue sizes 
(16kB, 64kB, 256kB and 1MB) and according to the results the queue size 
really makes almost no difference. It might make a difference for some 
queries, but I wouldn't bother tuning this until we have a plausible 
example - increasing the queue size is not free either.

So it was worth checking, but I'd just leave it as 64kB for now. We may 
revisit this later in a separate patch/thread.

>     As for the patches, I think the proposed changes are sensible, but I
>     wonder what queries might get slower. For example with the batching
>     (updating the counter only once every 4kB, that pretty much transfers
>     data in larger chunks with higher latency. So what if the query needs
>     only a small chunk, like a LIMIT query? Similarly, this might mean the
>     upper parts of the plan have to wait for the data for longer, and thus
>     can't start some async operation (like send them to a FDW, or something
>     like that). I do admit those are theoretical queries, I haven't tried
>     creating such query.
> 
> 
> Yeah, I was thinking about such cases, basically, this design can 
> increase the startup cost of the Gather node, I will also try to derive 
> such cases and test them.
> 
> 
>     FWIW I've tried applying both patches at the same time, but there's a
>     conflict in shm_mq_sendv - not a complex one, but I'm not sure what's
>     the correct solution. Can you share a "combined" patch?
> 
> 
> Actually, these both patches are the same, 
> "v1-0001-Optimize-parallel-tuple-send-shm_mq_send_bytes.patch" is the 
> cleaner version of the first patch.  For configurable tuple queue size I 
> did not send a patch, because that is I just used for the testing 
> purpose and never intended to to propose anything.  My most of the 
> latest performance data I sent with only 
> "v1-0001-Optimize-parallel-tuple-send-shm_mq_send_bytes.patch" and with 
> default tuple queue size.
> 
> But I am attaching both the patches in case you want to play around.
> 

Ah, silly me. I should have noticed the second patch is just a refined 
version of the first one.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company