This is random thoughts on this patch.
At Tue, 30 Aug 2016 12:17:52 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in
<20160830.121752.100817694.horiguchi.kyotaro@lab.ntt.co.jp>
> > As the result, the attached patchset is functionally the same
> > with the last version but replace misused Assert with
> > AssertMacro.
There's perfomance degradation for non-asynchronous nodes, as
shown as 't0' below.
The patch adds two "if-then" and one additional function call as
asynchronous stuff into ExecProcnode, which is heavily passed and
foremerly consists only five meaningful lines. The stuff slows
performance by about 1% for simple seqscan case. The following is
the performance numbers previously shown upthread. (Or the
difference might be too small to get meaningful performance
difference..)
===
t0- (SeqScan()) (2 parallel)
pl- (Append(4 * SeqScan()))
pf0 (Append(4 * ForeignScan())) all ForeignScans are on the same connection.
pf1 (Append(4 * ForeignScan())) all ForeignScans have their own connections.
patched-O2 time(ms) stddev(ms) gain from unpatched (%) t0 4121.27 1.1 -1.44 pl
1757.41 0.94 -1.73 pf0 6458.99 192.4 20.26 pf1 1747.4 24.81 78.39
unpatched-O2 t0 4062.6 1.95 pl 1727.45 9.41
pf0 8100.47 24.51 pf1 8086.52 33.53
===
So, finally, it seems that the infrastructure should not habit in
ExecProcNode, or need to redesign the executor. I tried
jump-table to dispatch nodes which was in vain. Having a flag in
EState may be able to get rid of async stuff from non-async
route. (similar to, but maybe different from my first patch) JIT
compiling seems promising but it is a different thing.
As for nodeGather, it expects the leader process to be one of
workers, the leader should be free from it so as to behave as an
async node. But still the expectected number of workers seems to
be too small to take a meaningful benefit from async execution.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center