From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <amitlangote09@gmail.com>
cc: Julien Rouhaud <rjuju123@gmail.com>,
        Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>,
        Joe Conway <mail@joeconway.com>,
        David Rowley <david.rowley@2ndquadrant.com>,
        Andreas Seltenreich <andreas.seltenreich@credativ.de>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Excessive memory usage in multi-statement queries w/ partitioning
In-reply-to: 
 <CA+HiwqEwK-rZMt3TY_FL4jPKOSFJhH_FzAdW8XXNZx4Er753sQ@mail.gmail.com>
References: <87ftp6l2qr.fsf@credativ.de>
 <e0ec915a-ebd3-ee9d-144b-e24bc3fae3c4@lab.ntt.co.jp>
 <0e717312-3a02-781b-7dfc-e0cc395e92d8@joeconway.com>
 <CAKJS1f-StHT8ZKyC8rsK=nJim13qDWDiNO8NMirTBo_jk=z5Aw@mail.gmail.com>
 <db7efe35-03eb-e715-9766-d564dc479f0b@joeconway.com>
 <6014.1558708103@sss.pgh.pa.us>
 <3ed13306-ba63-f2e8-1eeb-63b0e4f377e6@lab.ntt.co.jp>
 <65092.1558961789@sss.pgh.pa.us>
 <4bea720a-4310-36e9-b6ae-886669bceeef@lab.ntt.co.jp>
 <CAOBaU_ZAQiJpZ0YRoe+zUnJ0d1+Yd656oRtcSRQvx+WmNiWE7Q@mail.gmail.com>
 <CA+HiwqEwK-rZMt3TY_FL4jPKOSFJhH_FzAdW8XXNZx4Er753sQ@mail.gmail.com>
Comments: In-reply-to Amit Langote <amitlangote09@gmail.com>
	message dated "Mon, 08 Jul 2019 17:52:20 +0900"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3603.1562620851.1@sss.pgh.pa.us>
Date: Mon, 08 Jul 2019 17:20:51 -0400
Message-ID: <3604.1562620851@sss.pgh.pa.us>
Precedence: bulk

Amit Langote <amitlangote09@gmail.com> writes:
> [ parse-plan-memcxt_v2.patch ]

I got around to looking at this finally.  I'm not at all happy with
the fact that it's added a plantree copy step to the only execution
path through exec_simple_query.  That's a very significant overhead,
in many use-cases, to solve something that nobody had complained
about for a couple of decades before now.  I don't see the need for
any added copy step anyway.  The only reason you're doing it AFAICS
is so you can release the per-statement context a bit earlier, which
is a completely unnecessary optimization.  Just wait to release it
till the bottom of the loop.

Also, creating/deleting the sub-context is in itself an added overhead
that accomplishes exactly nothing in the typical case where there's
not multiple statements.  I thought the idea was to do that only if
there was more than one raw parsetree (or, maybe better, do it for
all but the last parsetree).

To show that this isn't an empty concern, I did a quick pgbench
test.  Using a single-client select-only test ("pgbench -S -T 60"
in an -s 10 database), I got these numbers in three trials with HEAD:

tps = 9593.818478 (excluding connections establishing)
tps = 9570.189163 (excluding connections establishing)
tps = 9596.579038 (excluding connections establishing)

and these numbers after applying the patch:

tps = 9411.918165 (excluding connections establishing)
tps = 9389.279079 (excluding connections establishing)
tps = 9409.350175 (excluding connections establishing)

That's about a 2% dropoff.  Now it's possible that that can be
explained away as random variations from a slightly different layout
of critical loops vs cacheline boundaries ... but I don't believe it.

			regards, tom lane