Re: Removed extra memory allocations from create_list_bounds

> 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(at)telsasoft(dot)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(at)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
>