From: Tom Lane <tgl@sss.pgh.pa.us>
To: Claudio Freire <klaussfreire@gmail.com>
Cc: Sokolov Yura <funny.falcon@postgrespro.ru>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: PostgreSQL-Dev <pgsql-hackers@postgresql.org>
Subject: Re: Small improvement to compactify_tuples
Date: Fri, 03 Nov 2017 15:30:25 -0400
Message-ID: <11367.1509737425@sss.pgh.pa.us> (raw)
In-Reply-To: <CAGTBQpYtaiZfN+=EExrm_oXvypMmKeEjue33ZkzGohDr-jbQyA@mail.gmail.com>
References: <20170515171331.taa44gjbc7p3aaul@alvherre.pgsql>
<468f371fe05c1de09c4dc101435ef93d@postgrespro.ru>
<9c4bc2ff302be669717101bc33127fd0@postgrespro.ru>
<a9327865736cd63820f009141917063f@postgrespro.ru>
<CAGTBQpbrCqPLTfM10TPb3G+DXwDzv=5zQzhuDs9RJV1LUK0+WA@mail.gmail.com>
<6873f723fd9cfed7389a2f461016e167@postgrespro.ru>
<21392.1509677196@sss.pgh.pa.us>
<CAGTBQpYtaiZfN+=EExrm_oXvypMmKeEjue33ZkzGohDr-jbQyA@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
Claudio Freire <klaussfreire@gmail.com> writes:
> On Thu, Nov 2, 2017 at 11:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> BTW, the originally given test case shows no measurable improvement
>> on my box.
> I did manage to reproduce the original test and got a consistent improvement.
It occurred to me that I could force the issue by hacking bufpage.c to
execute compactify_tuples multiple times each time it was called, as in
the first patch attached below. This has nothing directly to do with real
performance of course, but it's making use of the PG system to provide
realistic test data for microbenchmarking compactify_tuples. I was a bit
surprised to find that I had to set the repeat count to 1000 to make
compactify_tuples really dominate the runtime (while using the originally
posted test case ... maybe there's a better one?). But once I did get it
to dominate the runtime, perf gave me this for the CPU hotspots:
+ 27.97% 27.88% 229040 postmaster libc-2.12.so [.] memmove
+ 14.61% 14.57% 119704 postmaster postgres [.] compactify_tuples
+ 12.40% 12.37% 101566 postmaster libc-2.12.so [.] _wordcopy_bwd_aligned
+ 11.68% 11.65% 95685 postmaster libc-2.12.so [.] _wordcopy_fwd_aligned
+ 7.67% 7.64% 62801 postmaster postgres [.] itemoffcompare
+ 7.00% 6.98% 57303 postmaster postgres [.] compactify_tuples_loop
+ 4.53% 4.52% 37111 postmaster postgres [.] pg_qsort
+ 1.71% 1.70% 13992 postmaster libc-2.12.so [.] memcpy
which says that micro-optimizing the sort step is a complete, utter waste
of time, and what we need to be worried about is the data copying part.
The memcpy part of the above is presumably from the scaffolding memcpy's
in compactify_tuples_loop, which is interesting because that's moving as
much data as the memmove's are. So at least with RHEL6's version of
glibc, memmove is apparently a lot slower than memcpy.
This gave me the idea to memcpy the page into some workspace and then use
memcpy, not memmove, to put the tuples back into the caller's copy of the
page. That gave me about a 50% improvement in observed TPS, and a perf
profile like this:
+ 38.50% 38.40% 299520 postmaster postgres [.] compactify_tuples
+ 31.11% 31.02% 241975 postmaster libc-2.12.so [.] memcpy
+ 8.74% 8.72% 68022 postmaster postgres [.] itemoffcompare
+ 6.51% 6.49% 50625 postmaster postgres [.] compactify_tuples_loop
+ 4.21% 4.19% 32719 postmaster postgres [.] pg_qsort
+ 1.70% 1.69% 13213 postmaster postgres [.] memcpy@plt
There still doesn't seem to be any point in replacing the qsort,
but it does seem like something like the second attached patch
might be worth doing.
So I'm now wondering why my results seem to be so much different
from those of other people who have tried this, both as to whether
compactify_tuples is worth working on at all and as to what needs
to be done to it if so. Thoughts?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:
[text/x-diff] repeat-compactify.patch (1.8K, ../11367.1509737425@sss.pgh.pa.us/2-repeat-compactify.patch)
download | inline diff:diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.cindex 41642eb..bf6d308 100644
*** a/src/backend/storage/page/bufpage.c
--- b/src/backend/storage/page/bufpage.c
*************** compactify_tuples(itemIdSort itemidbase,
*** 465,470 ****
--- 465,489 ----
phdr->pd_upper = upper;
}
+ static void+ compactify_tuples_loop(itemIdSort itemidbase, int nitems, Page page)+ {+ itemIdSortData copy[Max(MaxIndexTuplesPerPage, MaxHeapTuplesPerPage)];+ union {+ char page[BLCKSZ];+ double align;+ } pagecopy;+ int i;+ + for (i = 1; i < 1000; i++)+ {+ memcpy(copy, itemidbase, sizeof(itemIdSortData) * nitems);+ memcpy(pagecopy.page, page, BLCKSZ);+ compactify_tuples(copy, nitems, pagecopy.page);+ }+ compactify_tuples(itemidbase, nitems, page);+ }+
/*
* PageRepairFragmentation
*
*************** PageRepairFragmentation(Page page)
*** 560,566 ****
errmsg("corrupted item lengths: total %u, available space %u",
(unsigned int) totallen, pd_special - pd_lower)));
! compactify_tuples(itemidbase, nstorage, page);
}
/* Set hint bit for PageAddItem */
--- 579,585 ----
errmsg("corrupted item lengths: total %u, available space %u",
(unsigned int) totallen, pd_special - pd_lower)));
! compactify_tuples_loop(itemidbase, nstorage, page);
}
/* Set hint bit for PageAddItem */
*************** PageIndexMultiDelete(Page page, OffsetNu
*** 940,946 ****
phdr->pd_lower = SizeOfPageHeaderData + nused * sizeof(ItemIdData);
/* and compactify the tuple data */
! compactify_tuples(itemidbase, nused, page);
}
--- 959,965 ----
phdr->pd_lower = SizeOfPageHeaderData + nused * sizeof(ItemIdData);
/* and compactify the tuple data */
! compactify_tuples_loop(itemidbase, nused, page);
}
[text/x-diff] use-memcpy-not-memmove.patch (1.5K, ../11367.1509737425@sss.pgh.pa.us/3-use-memcpy-not-memmove.patch)
download | inline diff:diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.cindex 41642eb..e485398 100644
*** a/src/backend/storage/page/bufpage.c
--- b/src/backend/storage/page/bufpage.c
*************** static void
*** 441,446 ****
--- 441,450 ----
compactify_tuples(itemIdSort itemidbase, int nitems, Page page)
{
PageHeader phdr = (PageHeader) page;
+ union {+ char page[BLCKSZ];+ double align;+ } pagecopy;
Offset upper;
int i;
*************** compactify_tuples(itemIdSort itemidbase,
*** 448,464 ****
qsort((char *) itemidbase, nitems, sizeof(itemIdSortData),
itemoffcompare);
upper = phdr->pd_special;
for (i = 0; i < nitems; i++)
{
itemIdSort itemidptr = &itemidbase[i];
ItemId lp;
- lp = PageGetItemId(page, itemidptr->offsetindex + 1);
upper -= itemidptr->alignedlen;
! memmove((char *) page + upper,
! (char *) page + itemidptr->itemoff,
! itemidptr->alignedlen);
lp->lp_off = upper;
}
--- 452,470 ----
qsort((char *) itemidbase, nitems, sizeof(itemIdSortData),
itemoffcompare);
+ memcpy(pagecopy.page, page, BLCKSZ);+
upper = phdr->pd_special;
for (i = 0; i < nitems; i++)
{
itemIdSort itemidptr = &itemidbase[i];
ItemId lp;
upper -= itemidptr->alignedlen;
! memcpy((char *) page + upper,
! pagecopy.page + itemidptr->itemoff,
! itemidptr->alignedlen);
! lp = PageGetItemId(page, itemidptr->offsetindex + 1);
lp->lp_off = upper;
}
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, klaussfreire@gmail.com, funny.falcon@postgrespro.ru, alvherre@2ndquadrant.com, hlinnaka@iki.fi
Subject: Re: Small improvement to compactify_tuples
In-Reply-To: <11367.1509737425@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox