Re: Changing types of block and chunk sizes in memory contexts

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Rowley <dgrowleyml(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Changing types of block and chunk sizes in memory contexts
Date: 2023-06-29 09:58:27
Message-ID: f9db1b33-2713-214f-9a18-c0e07d706b8e@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 6/29/23 01:34, Andres Freund wrote:
> Hi,
>
> On 2023-06-28 23:26:00 +0200, Tomas Vondra wrote:
>> Yeah. FWIW I was interested what the patch does in practice, so I
>> checked what pahole says about impact on struct sizes:
>>
>> AllocSetContext 224B -> 208B (4 cachelines)
>> GenerationContext 152B -> 136B (3 cachelines)
>> SlabContext 200B -> 200B (no change, adds 4B hole)
>>
>> Nothing else changes, AFAICS. I find it hard to believe this could have
>> any sort of positive benefit - I doubt we ever have enough contexts for
>> this to matter.
>
> I don't think it's that hard to believe. We create a lot of memory contexts
> that we never or just barely use. Just reducing the number of cachelines
> touched for that can't hurt. This does't quite get us to reducing the size to
> a lower number of cachelines, but it's a good step.
>
> There are a few other fields that we can get rid of.
>
> - Afaics AllocSet->keeper is unnecessary these days, as it is always allocated
> together with the context itself. Saves 8 bytes.
>
> - The set of memory context types isn't runtime extensible. We could replace
> MemoryContextData->methods with a small integer index into mcxt_methods. I
> think that might actually end up being as-cheap or even cheaper than the
> current approach. Saves 8 bytes.
>
> Tthat's sufficient for going to 3 cachelines.
>
>
> - We could store the power of 2 for initBlockSize, nextBlockSize,
> maxBlockSize, instead of the "raw" value. That'd make them one byte
> each. Which also would get rid of the concerns around needing a
> "mini_size_t" type.
>
> - freeListIndex could be a single byte as well (saving 7 bytes, as right now
> we loose 4 trailing bytes due to padding).
>
> That would save another 12 bytes, if I calculate correctly. 25% shrinkage
> together ain't bad.
>

I don't oppose these changes, but I still don't quite believe it'll make
a measurable difference (even if we manage to save a cacheline or two).
I'd definitely like to see some measurements demonstrating it's worth
the extra complexity.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message jian he 2023-06-29 10:20:32 Re: Incremental View Maintenance, take 2
Previous Message Alena Rybakina 2023-06-29 09:55:58 Re: POC, WIP: OR-clause support for indexes