Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eAhhY-0007Iw-Je for pgsql-hackers@arkaria.postgresql.org; Fri, 03 Nov 2017 19:32:24 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1eAhhX-0004A3-CT for pgsql-hackers@arkaria.postgresql.org; Fri, 03 Nov 2017 19:32:23 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1eAhfy-0000yx-Rf for pgsql-hackers@postgresql.org; Fri, 03 Nov 2017 19:30:46 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1eAhfv-0004du-H2 for pgsql-hackers@postgresql.org; Fri, 03 Nov 2017 19:30:45 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id vA3JUPMl011368; Fri, 3 Nov 2017 15:30:25 -0400 From: Tom Lane To: Claudio Freire cc: Sokolov Yura , Alvaro Herrera , Heikki Linnakangas , PostgreSQL-Dev Subject: Re: Small improvement to compactify_tuples In-reply-to: References: <20170515171331.taa44gjbc7p3aaul@alvherre.pgsql> <468f371fe05c1de09c4dc101435ef93d@postgrespro.ru> <9c4bc2ff302be669717101bc33127fd0@postgrespro.ru> <6873f723fd9cfed7389a2f461016e167@postgrespro.ru> <21392.1509677196@sss.pgh.pa.us> Comments: In-reply-to Claudio Freire message dated "Fri, 03 Nov 2017 11:37:46 -0300" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <11272.1509737277.0@sss.pgh.pa.us> Date: Fri, 03 Nov 2017 15:30:25 -0400 Message-ID: <11367.1509737425@sss.pgh.pa.us> List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgsql-hackers Precedence: bulk Sender: pgsql-hackers-owner@postgresql.org ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <11272.1509737277.1@sss.pgh.pa.us> Content-Transfer-Encoding: quoted-printable Claudio Freire writes: > On Thu, Nov 2, 2017 at 11:46 PM, Tom Lane 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 improvem= ent. 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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="repeat-compactify.patch"; charset="us-ascii" Content-ID: <11272.1509737277.2@sss.pgh.pa.us> Content-Description: repeat-compactify.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/= bufpage.c index 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 =3D upper; } =20=20 + 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; +=20 + for (i =3D 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); + } +=20 /* * PageRepairFragmentation * *************** PageRepairFragmentation(Page page) *** 560,566 **** errmsg("corrupted item lengths: total %u, available space %u", (unsigned int) totallen, pd_special - pd_lower))); =20=20 ! compactify_tuples(itemidbase, nstorage, page); } =20=20 /* Set hint bit for PageAddItem */ --- 579,585 ---- errmsg("corrupted item lengths: total %u, available space %u", (unsigned int) totallen, pd_special - pd_lower))); =20=20 ! compactify_tuples_loop(itemidbase, nstorage, page); } =20=20 /* Set hint bit for PageAddItem */ *************** PageIndexMultiDelete(Page page, OffsetNu *** 940,946 **** phdr->pd_lower =3D SizeOfPageHeaderData + nused * sizeof(ItemIdData); =20=20 /* and compactify the tuple data */ ! compactify_tuples(itemidbase, nused, page); } =20=20 =20=20 --- 959,965 ---- phdr->pd_lower =3D SizeOfPageHeaderData + nused * sizeof(ItemIdData); =20=20 /* and compactify the tuple data */ ! compactify_tuples_loop(itemidbase, nused, page); } =20=20 =20=20 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="use-memcpy-not-memmove.patch"; charset="us-ascii" Content-ID: <11272.1509737277.3@sss.pgh.pa.us> Content-Description: use-memcpy-not-memmove.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/= bufpage.c index 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 =3D (PageHeader) page; + union { + char page[BLCKSZ]; + double align; + } pagecopy; Offset upper; int i; =20=20 *************** compactify_tuples(itemIdSort itemidbase, *** 448,464 **** qsort((char *) itemidbase, nitems, sizeof(itemIdSortData), itemoffcompare); =20=20 upper =3D phdr->pd_special; for (i =3D 0; i < nitems; i++) { itemIdSort itemidptr =3D &itemidbase[i]; ItemId lp; =20=20 - lp =3D PageGetItemId(page, itemidptr->offsetindex + 1); upper -=3D itemidptr->alignedlen; ! memmove((char *) page + upper, ! (char *) page + itemidptr->itemoff, ! itemidptr->alignedlen); lp->lp_off =3D upper; } =20=20 --- 452,470 ---- qsort((char *) itemidbase, nitems, sizeof(itemIdSortData), itemoffcompare); =20=20 + memcpy(pagecopy.page, page, BLCKSZ); +=20 upper =3D phdr->pd_special; for (i =3D 0; i < nitems; i++) { itemIdSort itemidptr =3D &itemidbase[i]; ItemId lp; =20=20 upper -=3D itemidptr->alignedlen; ! memcpy((char *) page + upper, ! pagecopy.page + itemidptr->itemoff, ! itemidptr->alignedlen); ! lp =3D PageGetItemId(page, itemidptr->offsetindex + 1); lp->lp_off =3D upper; } =20=20 ------- =_aaaaaaaaaa0 Content-Type: text/plain Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers ------- =_aaaaaaaaaa0--