Re: Removed extra memory allocations from create_list_bounds - Mailing list pgsql-hackers

> I see this as a code cleanup more than an performance optimization.

I agree with this. This is like a code cleanup but it also improves performance.

I have done the performance testing, Just to confirm whether it really improves 

performance.

> 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.

I don't think that it will increase performance rather it adds to the improvement.

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

Thanks for sharing this. I have done the above settings and collected the

below data.

> 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.

In order to answer the above points, I have divided the patches into 2 sets.

1. Only 0001 and 0002 - These are related to list partitioning and do not contain 

changes related power-of-two allocation logic.

2. This contains all the 5 patches.

I have used the same testing procedure as explained in the previous mail.

Please find the timing information of the last 10 creation of partitioned

tables as given below.

Without patch	With 0001 and 0002	With all patch
17.105	14.722	13.878
15.897	14.427	13.493
15.991	15.424	14.751
17.965	16.487	19.491
19.704	19.042	21.278
18.98	18.949	18.123
18.986	21.713	17.585
21.273	20.596	19.623
18.839	18.521	17.605
20.724	18.774	19.242
18.5464	17.8655	17.5069

As we can see in the above data, there is an improvement with both of the

patch sets.

> 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;

Thanks a lot for sharing this information. It was really helpful.

I have collected the stat information for creation of 1000 

partitions. Please find the stat information for the 'without patch'

case below.

LOG:  EXECUTOR STATISTICS

DETAIL:  ! system usage stats:

! 0.000012 s user, 0.000000 s system, 0.000011 s elapsed

! [71.599426 s user, 4.362552 s system total]

! 63872 kB max resident size

! 0/0 [0/231096] filesystem blocks in/out

! 0/0 [0/2388074] page faults/reclaims, 0 [0] swaps

! 0 [0] signals rcvd, 0/0 [0/0] messages rcvd/sent

! 0/0 [9982/6709] voluntary/involuntary context switches

Please find the stat information for 'with patch (all 5 patches)' case below.

LOG:  EXECUTOR STATISTICS

DETAIL:  ! system usage stats:

! 0.000018 s user, 0.000002 s system, 0.000013 s elapsed

! [73.529715 s user, 4.219172 s system total]

! 63152 kB max resident size

! 0/0 [0/204840] filesystem blocks in/out

! 0/0 [0/2066377] page faults/reclaims, 0 [0] swaps

! 0 [0] signals rcvd, 0/0 [0/0] messages rcvd/sent

! 0/0 [9956/4129] voluntary/involuntary context switches

Please share your thoughts.

--

Thanks & Regards,

Nitin Jadhav

On Thu, May 20, 2021 at 12:47 AM Justin Pryzby <pryzby@telsasoft.com> wrote:

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