Message-ID: <53cd0f9541e3285e092b818ad19524584c5ac2d3.camel@postgrespro.ru>
Subject: Re: optimize lookups in snapshot [sub]xip arrays
From: Yura Sokolov <y.sokolov@postgrespro.ru>
To: Nathan Bossart <nathandbossart@gmail.com>, pgsql-hackers@postgresql.org
Date: Sun, 24 Jul 2022 15:26:12 +0300
In-Reply-To: <20220713170950.GA3116318@nathanxps13>
References: <20220713170950.GA3116318@nathanxps13>
Organization: PostgresPro
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.1-0ubuntu1 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/53cd0f9541e3285e092b818ad19524584c5ac2d3.camel%40postgrespro.ru>
Precedence: bulk

=D0=92 =D0=A1=D1=80, 13/07/2022 =D0=B2 10:09 -0700, Nathan Bossart =D0=BF=
=D0=B8=D1=88=D0=B5=D1=82:
> Hi hackers,
>=20
> A few years ago, there was a proposal to create hash tables for long
> [sub]xip arrays in snapshots [0], but the thread seems to have fizzled ou=
t.
> I was curious whether this idea still showed measurable benefits, so I
> revamped the patch and ran the same test as before [1].=C2=A0 Here are th=
e
> results for 60=E2=82=8Bsecond runs on an r5d.24xlarge with the data direc=
tory on
> the local NVMe storage:
>=20
> =C2=A0=C2=A0=C2=A0=C2=A0 writers=C2=A0 HEAD=C2=A0 patch=C2=A0 diff
> =C2=A0=C2=A0=C2=A0 ----------------------------
> =C2=A0=C2=A0=C2=A0=C2=A0 16=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 659=C2=A0=
=C2=A0 664=C2=A0=C2=A0=C2=A0 +1%
> =C2=A0=C2=A0=C2=A0=C2=A0 32=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 645=C2=A0=
=C2=A0 663=C2=A0=C2=A0=C2=A0 +3%
> =C2=A0=C2=A0=C2=A0=C2=A0 64=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 659=C2=A0=
=C2=A0 692=C2=A0=C2=A0=C2=A0 +5%
> =C2=A0=C2=A0=C2=A0=C2=A0 128=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 641=C2=A0=C2=
=A0 716=C2=A0=C2=A0=C2=A0 +12%
> =C2=A0=C2=A0=C2=A0=C2=A0 256=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 619=C2=A0=C2=
=A0 610=C2=A0=C2=A0=C2=A0 -1%
> =C2=A0=C2=A0=C2=A0=C2=A0 512=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 530=C2=A0=C2=
=A0 702=C2=A0=C2=A0=C2=A0 +32%
> =C2=A0=C2=A0=C2=A0=C2=A0 768=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 469=C2=A0=C2=
=A0 582=C2=A0=C2=A0=C2=A0 +24%
> =C2=A0=C2=A0=C2=A0=C2=A0 1000=C2=A0=C2=A0=C2=A0=C2=A0 367=C2=A0=C2=A0 577=
=C2=A0=C2=A0=C2=A0 +57%
>=20
> As before, the hash table approach seems to provide a decent benefit at
> higher client counts, so I felt it was worth reviving the idea.
>=20
> The attached patch has some key differences from the previous proposal.
> For example, the new patch uses simplehash instead of open-coding a new
> hash table.=C2=A0 Also, I've bumped up the threshold for creating hash ta=
bles to
> 128 based on the results of my testing.=C2=A0 The attached patch waits un=
til a
> lookup of [sub]xip before generating the hash table, so we only need to
> allocate enough space for the current elements in the [sub]xip array, and
> we avoid allocating extra memory for workloads that do not need the hash
> tables.=C2=A0 I'm slightly worried about increasing the number of memory
> allocations in this code path, but the results above seemed encouraging o=
n
> that front.
>=20
> Thoughts?
>=20
> [0] https://postgr.es/m/35960b8af917e9268881cd8df3f88320%40postgrespro.ru
> [1] https://postgr.es/m/057a9a95-19d2-05f0-17e2-f46ff20e9b3e%402ndquadran=
t.com
>=20

I'm glad my idea has been reborn.

Well, may be simplehash is not bad idea.
While it certainly consumes more memory and CPU instructions.

I'll try to review.

regards,

Yura Sokolov