From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Andres Freund <andres@anarazel.de>
Cc: Aleksander Alekseev <aleksander@timescale.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Julien Rouhaud <rjuju123@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Subject: Re: ResourceOwner refactoring
Date: Tue, 7 Nov 2023 13:24:48 +0200
Message-ID: <bbb3ebeb-cde9-48b4-8433-36d7ed41648e@iki.fi> (raw)
In-Reply-To: <20231025180729.dmsohuolcv4d5rqa@awork3.anarazel.de>
References: <20221101001523.lsgmohl6fjwppbgx@awork3.anarazel.de>
<9a3245db-ca03-4a47-5706-73404cfe5108@iki.fi>
<20221101154251.64vyq7wy6w7aykrn@awork3.anarazel.de>
<bf88d2a9-a7bc-3d68-57e3-abfba87832e9@iki.fi>
<CAJ7c6TNijrJ+0hnTiAPDBn-tCiJsk-yvpF3Y8rXG4gTw09JYbg@mail.gmail.com>
<d17d4d6b-c39c-7e40-9f3d-b09f19e5ccec@iki.fi>
<CAJ7c6TM_9XtQX6xAGkH2u2-b20z-1rii9szMdW-aMHJZC_NOsA@mail.gmail.com>
<527af829-4a03-03b0-121c-46ff79c61887@iki.fi>
<20230710191446.pn5ltm73t2i3xtxz@awork3.anarazel.de>
<20607b96-b275-4a57-8509-29a3ed320d3f@iki.fi>
<20231025180729.dmsohuolcv4d5rqa@awork3.anarazel.de>
On 25/10/2023 21:07, Andres Freund wrote:
> It's not too awful to have it be in this order:
>
> struct ResourceOwnerData {
> ResourceOwner parent; /* 0 8 */
> ResourceOwner firstchild; /* 8 8 */
> ResourceOwner nextchild; /* 16 8 */
> const char * name; /* 24 8 */
> _Bool releasing; /* 32 1 */
> _Bool sorted; /* 33 1 */
> uint8 narr; /* 34 1 */
> uint8 nlocks; /* 35 1 */
> uint32 nhash; /* 36 4 */
> uint32 capacity; /* 40 4 */
> uint32 grow_at; /* 44 4 */
> ResourceElem arr[32]; /* 48 512 */
> /* --- cacheline 8 boundary (512 bytes) was 48 bytes ago --- */
> ResourceElem * hash; /* 560 8 */
> LOCALLOCK * locks[15]; /* 568 120 */
>
> /* size: 688, cachelines: 11, members: 14 */
> /* last cacheline: 48 bytes */
> };
>
> Requires rephrasing a few comments to document that the lenghts are separate
> from the array / hashtable / locks, but otherwise...
Hmm, let's move 'capacity' and 'grow_at' after 'arr', too. They are only
needed together with 'hash'.
> This reliably shows a decent speed improvement in my stress test [1], on the
> order of 5%.
>
> [1] pgbench running
> DO $$ BEGIN FOR i IN 1..5000 LOOP PERFORM abalance FROM pgbench_accounts WHERE aid = 17;END LOOP;END;$$;
I'm seeing similar results, although there's enough noise in the test
that I'm sure how real the would be across different tests.
> At that point, the first array entry fits into the first cacheline. If we were
> to move parent, firstchild, nextchild, name further down the struct, three
> array entries would be on the first line. Just using the array presumably is a
> very common case, so that might be worth it?
>
> I got less clear performance results with this one, and it's also quite
> possible it could hurt, if resowners aren't actually "used", just
> released. Therefore it's probably not worth it for now.
You're assuming that the ResourceOwner struct begins at a cacheline
boundary. That's not usually true, we don't try to cacheline-align it.
So while it's helpful to avoid padding and to keep frequently-accessed
fields close to each other, there's no benefit in keeping them at the
beginning of the struct.
--
Heikki Linnakangas
Neon (https://neon.tech)
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: hlinnaka@iki.fi, andres@anarazel.de, aleksander@timescale.com, rjuju123@gmail.com, robertmhaas@gmail.com
Subject: Re: ResourceOwner refactoring
In-Reply-To: <bbb3ebeb-cde9-48b4-8433-36d7ed41648e@iki.fi>
* 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