MIME-Version: 1.0
References: <CALj2ACXdrOmB6Na9amHWZHKvRT3Z0nwTRsCwoMT-npOBtmXLXg@mail.gmail.com>
 <20230603223824.o7iyochli2dwwi7k@alap3.anarazel.de> <CALj2ACX5UMWVFdrRNUE0KDrg54WV1cumBXwcETXhrPc1ibKAQA@mail.gmail.com>
 <CAAWbhmj5Pio3nOUakObzLGCSS9dwFfgsNVDhwTGzXNwZc00uCQ@mail.gmail.com>
 <CALj2ACVHC=c6eC9SRxhcTUrnXvNDNkEBgedi2WkVJYRb=0sWYw@mail.gmail.com>
 <CALj2ACVE2h=LnFnpr3rh+6SZzdwzW5EZOYG2Z0t=p28Fn75eag@mail.gmail.com>
 <CALj2ACWT0Rz8oybWBm5W4CeS0DvFkwaw-pEvGArhDLyPbZnW_g@mail.gmail.com>
 <CALj2ACWxO3HPtpYZb765LZk-uKVuAvZPO1HDeZ8=mzMgVPgaww@mail.gmail.com> <CALj2ACXJA4QQ_6zAHez0Uy-9t-ebmpox2y1QBja+mF4QP+h8WQ@mail.gmail.com>
In-Reply-To: <CALj2ACXJA4QQ_6zAHez0Uy-9t-ebmpox2y1QBja+mF4QP+h8WQ@mail.gmail.com>
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 19 Mar 2024 14:09:36 +0900
Message-ID: <CAD21AoD97mhzF8cqsd2v1jg9z8xfvAJrPx6Wvi+Ev0Hmu96LJA@mail.gmail.com>
Subject: Re: New Table Access Methods for Multi and Single Inserts
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>, 
	Dilip Kumar <dilipbalaut@gmail.com>, Luc Vlaming <luc@swarm64.com>, 
	Justin Pryzby <pryzby@telsasoft.com>, Jeff Davis <pgsql@j-davis.com>, 
	Michael Paquier <michael@paquier.xyz>, Matthias van de Meent <boekewurm+postgres@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAD21AoD97mhzF8cqsd2v1jg9z8xfvAJrPx6Wvi%2BEv0Hmu96LJA%40mail.gmail.com>
Precedence: bulk

Hi,

On Fri, Mar 8, 2024 at 7:37=E2=80=AFPM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Sat, Mar 2, 2024 at 12:02=E2=80=AFPM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > On Mon, Jan 29, 2024 at 5:16=E2=80=AFPM Bharath Rupireddy
> > <bharath.rupireddyforpostgres@gmail.com> wrote:
> > >
> > > > Please find the attached v9 patch set.
> >
> > I've had to rebase the patches due to commit 874d817, please find the
> > attached v11 patch set.
>
> Rebase needed. Please see the v12 patch set.
>

I've not reviewed the patches in depth yet, but run performance tests
for CREATE MATERIALIZED VIEW. The test scenarios is:

-- setup
create unlogged table test (c int);
insert into test select generate_series(1, 10000000);

-- run
create materialized view test_mv as select * from test;

Here are the results:

* HEAD
3775.221 ms
3744.039 ms
3723.228 ms

* v12 patch
6289.972 ms
5880.674 ms
7663.509 ms

I can see performance regressions and the perf report says that CPU
spent most time on extending the ResourceOwner's array while copying
the buffer-heap tuple:

- 52.26% 0.18% postgres postgres [.] intorel_receive
    52.08% intorel_receive
        table_multi_insert_v2 (inlined)
        - heap_multi_insert_v2
            - 51.53% ExecCopySlot (inlined)
                tts_buffer_heap_copyslot
                tts_buffer_heap_store_tuple (inlined)
             - IncrBufferRefCount
                 - ResourceOwnerEnlarge
                     ResourceOwnerAddToHash (inlined)

Is there any reason why we copy a buffer-heap tuple to another
buffer-heap tuple? Which results in that we increments the buffer
refcount and register it to ResourceOwner for every tuples. I guess
that the destination tuple slot is not necessarily a buffer-heap, and
we could use VirtualTupleTableSlot instead. It would in turn require
copying a heap tuple. I might be missing something but it improved the
performance at least in my env. The change I made was:

-       dstslot =3D table_slot_create(state->rel, NULL);
+       //dstslot =3D table_slot_create(state->rel, NULL);
+       dstslot =3D MakeTupleTableSlot(RelationGetDescr(state->rel),
+                                    &TTSOpsVirtual);
+

And the execution times are:
1588.984 ms
1591.618 ms
1582.519 ms

Regards,

--=20
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com