Re: segfault in HEAD when too many nested functions call

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: segfault in HEAD when too many nested functions call
Date: 2017-07-17 14:57:10
Message-ID: 20170717145710.cm32m4yn555hsoxd@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2017-07-17 00:26:29 -0700, Andres Freund wrote:
> I'm less convinced of that, due to the overhead argument. I think
> there's a couple ways around that however:
>
> 1) We could move ExecProcNode() to be callback based. The first time a
> node is executed a "wrapper" function is called that does the stack
> and potentially other checks. That also makes ExecProcNode() small
> enough to be inlined, which ends up being good for jump target
> prediction. I've done something similar for v11 for expression
> evaluation, getting rid of EEOP_*_FIRST duplication etc, and it seems
> to work well. The big disadvantage to that is that it's a bit
> invasive for v10, and very likely too invasive to backpatch.
> 2) I think there's some fair argument to be made that ExecInitNode()'s
> stack-space needs are similar enough to ExecProcNode()'s allowing us
> to put a check_stack_depth() into the former. That seems like it's
> required anyway, since in many cases that's going to trigger
> stack-depth exhaustion first anyway (unless we hit it in parse
> analysis, which also seems quite common).

Attached is a trivial patch implementing 1) and a proof-of-concept hack
for 2).

The latter obviously isn't ready, but might make clearer what I'm
thinking about. If we were to go this route we'd have to probably move
the callback assignment into the ExecInit* routines, and possibly
replace the switch in ExecInitNode() with another callback, assigned in
make_*, and implement callbacks for ExecShutdown, MultiExecProcNode etc.

This results in a good speedup in tpc-h btw:
master total min: 46434.897 cb min: 44778.228 [diff -3.70]

- Andres

Attachment Content-Type Size
0001-Check-stack-depth-when-initializing-executor-nodes.patch text/x-patch 1.9 KB
0002-WIP-Move-to-callback-based-executor-nodes.patch text/x-patch 19.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2017-07-17 15:00:02 Re: AdvanceXLInsertBuffer vs. WAL segment compressibility
Previous Message Andrew Dunstan 2017-07-17 14:48:07 Unportable use of select for timeouts in PostgresNode.pm