MIME-Version: 1.0
References: <CADUqk8Uqw5QaUqLdd-0SBCvZVncrE3JMJB9+yDwO_uMv_hTYCg@mail.gmail.com>
 <20230217003204.cx5zu2aocjbwz5ns@awork3.anarazel.de> <CADUqk8UoG-zg-HEjF5xhPi5=j9h5=_BdCvxWwp1K8EKavbZe0g@mail.gmail.com>
 <20230217034000.ddcklsr6hfdifyqk@awork3.anarazel.de> <CAApHDvrbaGtG5CXRmh0aK5O8EAhxJGwGGXu9=m9GRLOQRkPTkw@mail.gmail.com>
In-Reply-To: <CAApHDvrbaGtG5CXRmh0aK5O8EAhxJGwGGXu9=m9GRLOQRkPTkw@mail.gmail.com>
From: "Jonah H. Harris" <jonah.harris@gmail.com>
Date: Thu, 16 Feb 2023 23:40:18 -0500
Message-ID: <CADUqk8VGdO8sm_Jd1CcvnfCiq2TzrZxxNTMD+Dv1-ytt-79EHA@mail.gmail.com>
Subject: Re: Reducing System Allocator Thrashing of ExecutorState to Alleviate
 FDW-related Performance Degradations
To: David Rowley <dgrowleyml@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000b39d8105f4dde9ca"
Archived-At: <https://www.postgresql.org/message-id/CADUqk8VGdO8sm_Jd1CcvnfCiq2TzrZxxNTMD%2BDv1-ytt-79EHA%40mail.gmail.com>
Precedence: bulk

--000000000000b39d8105f4dde9ca
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 16, 2023 at 11:26 PM David Rowley <dgrowleyml@gmail.com> wrote:

> I didn't hear it mentioned explicitly here, but I suspect it's faster
> when increasing the initial size due to the memory context caching
> code that reuses aset MemoryContexts (see context_freelists[] in
> aset.c). Since we reset the context before caching it, then it'll
> remain fast when we can reuse a context, provided we don't need to do
> a malloc for an additional block beyond the initial block that's kept
> in the cache.


This is what we were seeing. The larger initial size reduces/eliminates the
multiple smaller blocks that are malloced and freed in each per-query
execution.

Maybe we should think of a more general-purpose way of doing this
> caching which just keeps a global-to-the-process dclist of blocks
> laying around.  We could see if we have any free blocks both when
> creating the context and also when we need to allocate another block.
> I see no reason why this couldn't be shared among the other context
> types rather than keeping this cache stuff specific to aset.c.  slab.c
> might need to be pickier if the size isn't exactly what it needs, but
> generation.c should be able to make use of it the same as aset.c
> could.  I'm unsure what'd we'd need in the way of size classing for
> this, but I suspect we'd need to pay attention to that rather than do
> things like hand over 16MBs of memory to some context that only wants
> a 1KB initial block.


Yeah. There=E2=80=99s definitely a smarter and more reusable approach than =
I was
proposing. A lot of that code is fairly mature and I figured more people
wouldn=E2=80=99t want to alter it in such ways - but I=E2=80=99m up for it =
if an approach
like this is the direction we=E2=80=99d want to go in.


--=20
Jonah H. Harris

--000000000000b39d8105f4dde9ca
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div>On Thu, Feb 16, 2023 at 11:26 PM David Rowley &lt;<a href=3D"mailto:dg=
rowleyml@gmail.com">dgrowleyml@gmail.com</a>&gt; wrote:<br></div><div><div =
class=3D"gmail_quote"><blockquote class=3D"gmail_quote" style=3D"margin:0px=
 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1=
ex;border-left-color:rgb(204,204,204)" dir=3D"auto">I didn&#39;t hear it me=
ntioned explicitly here, but I suspect it&#39;s faster<br>
when increasing the initial size due to the memory context caching<br>
code that reuses aset MemoryContexts (see context_freelists[] in<br>
aset.c). Since we reset the context before caching it, then it&#39;ll<br>
remain fast when we can reuse a context, provided we don&#39;t need to do<b=
r>
a malloc for an additional block beyond the initial block that&#39;s kept<b=
r>
in the cache.</blockquote><div dir=3D"auto"><br></div><div dir=3D"auto">Thi=
s is what we were seeing. The larger initial size reduces/eliminates the mu=
ltiple smaller blocks that are malloced and freed in each per-query executi=
on.</div><div dir=3D"auto"><br></div><blockquote class=3D"gmail_quote" styl=
e=3D"margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid=
;padding-left:1ex;border-left-color:rgb(204,204,204)" dir=3D"auto">Maybe we=
 should think of a more general-purpose way of doing this<br>
caching which just keeps a global-to-the-process dclist of blocks<br>
laying around.=C2=A0 We could see if we have any free blocks both when<br>
creating the context and also when we need to allocate another block.<br>
I see no reason why this couldn&#39;t be shared among the other context<br>
types rather than keeping this cache stuff specific to aset.c.=C2=A0 slab.c=
<br>
might need to be pickier if the size isn&#39;t exactly what it needs, but<b=
r>
generation.c should be able to make use of it the same as aset.c<br>
could.=C2=A0 I&#39;m unsure what&#39;d we&#39;d need in the way of size cla=
ssing for<br>
this, but I suspect we&#39;d need to pay attention to that rather than do<b=
r>
things like hand over 16MBs of memory to some context that only wants<br>
a 1KB initial block.</blockquote><div dir=3D"auto"><br></div><div dir=3D"au=
to">Yeah. There=E2=80=99s definitely a smarter and more reusable approach t=
han I was proposing. A lot of that code is fairly mature and I figured more=
 people wouldn=E2=80=99t want to alter it in such ways - but I=E2=80=99m up=
 for it if an approach like this is the direction we=E2=80=99d want to go i=
n.</div><div dir=3D"auto"><br></div><div dir=3D"auto"><br></div><div dir=3D=
"auto"><br></div></div></div>-- <br><div dir=3D"ltr" class=3D"gmail_signatu=
re" data-smartmail=3D"gmail_signature"><div dir=3D"ltr"><div>Jonah H. Harri=
s<br></div><div><br></div></div></div>

--000000000000b39d8105f4dde9ca--