Date: Wed, 19 May 2021 14:16:58 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Zhihong Yu <zyu@yugabyte.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Removed extra memory allocations from create_list_bounds
Message-ID: <20210519191658.GA3496@telsasoft.com>
References: 
 <CAMm1aWYFTqEio3bURzZh47jveiHRwgQTiSDvBORczNEz2duZ1Q@mail.gmail.com>
 <20210516170029.GB373@telsasoft.com>
 <CALNJ-vS4g3QNmPe8WcgBmJQOzS0K_OAujW_1T0J2JXqpBQoBFw@mail.gmail.com>
 <CAMm1aWYr9=qPCTeNzVCEbF=Nr=RLSnt_vdCDTm9VjLS5L5JuOw@mail.gmail.com>
 <20210517150310.GG373@telsasoft.com>
 <CAMm1aWb0KSOCHexbaeit7gy_p4Uhs18EWyBdPyis4bfw4ids7A@mail.gmail.com>
 <CA+TgmoYgF_mPLMkRhHy=6GvYMrhATAB3SY4HNNwR0exY5J3EDQ@mail.gmail.com>
 <CAMm1aWZPaxwbk9gibFaEo30dAjMwFJ_meSY3BNDnHq6cOmQ67g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAMm1aWZPaxwbk9gibFaEo30dAjMwFJ_meSY3BNDnHq6cOmQ67g@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20210519191658.GA3496%40telsasoft.com>
Precedence: bulk

On Tue, May 18, 2021 at 01:49:12PM -0400, Robert Haas wrote:
> I see that you have made a theoretical argument for why this should be
> good for performance, but it would be better to have some test results
> that show that it works out in practice. Sometimes things seem like
> they ought to be more efficient but turn out to be less efficient when
> they are actually tried.

I see this as a code cleanup more than an performance optimization.
I couldn't see a measurable difference in my tests, involving CREATE TABLE and
SELECT.

I think some of my patches could *increase* memory use, due to power-of-two
allocation logic.  I think it's still a good idea, since it doesn't seem to be
the dominant memory allocation.

On Thu, May 20, 2021 at 12:21:19AM +0530, Nitin Jadhav wrote:
> > I see that you have made a theoretical argument for why this should be
> > good for performance, but it would be better to have some test results
> > that show that it works out in practice. Sometimes things seem like
> > they ought to be more efficient but turn out to be less efficient when
> > they are actually tried.
> 
> After this I tried to create 10 partitions and observed the time taken
> to execute. Here is the data for 'with patch'.
> 
> postgres@34077=#select 'create table t_' || i || ' partition of t for
> postgres'# values in (' || i || ');'
> postgres-# from generate_series(10001, 10010) i
> postgres-# \gexec

I think you should be sure to do this within a transaction, without cassert,
and maybe with fsync=off full_page_writes=off.

> If we observe above data, we can see the improvement with the patch.
> The average time taken to execute for the last 10 partitions is.

It'd be interesting to know which patch(es) contributed to the improvement.
It's possible that (say) patch 0001 alone gives all the gain, or that 0002
diminishes the gains.

I think there'll be an interest in committing the smallest possible patch to
realize the gains, to minimize code churn an unrelated changes.

LIST and RANGE might need to be checked separately..

> With respect to memory usage, AFAIK the allocated memory gets cleaned
> during deallocation of the memory used by the memory context. So at
> the end of the query, we may see no difference in the memory usage but
> during the query execution it tries to get less memory than before.

You can check MAXRSS (at least on linux) if you enable log_executor_stats,
like:

\set QUIET \\ SET client_min_messages=debug; SET log_executor_stats=on; DROP TABLE p; CREATE TABLE p(i int) PARTITION BY LIST(i); CREATE TABLE pd PARTITION OF p DEFAULT;
SELECT format('CREATE TABLE p%s PARTITION OF p FOR VALUES IN (%s)', a,a) FROM generate_series(1,999)a;\gexec \\ SELECT;

-- 
Justin