Re: Removed extra memory allocations from create_list_bounds

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

Sorry. Kindly ignore the above comment. I had misunderstood the statement.

Thanks & Regards,
Nitin Jadhav

On Sun, May 23, 2021 at 10:40 PM Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>
wrote:

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