Re: Changing types of block and chunk sizes in memory contexts - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: Changing types of block and chunk sizes in memory contexts
Date
Msg-id f9db1b33-2713-214f-9a18-c0e07d706b8e@enterprisedb.com
Whole thread Raw
In response to Re: Changing types of block and chunk sizes in memory contexts  (Andres Freund <andres@anarazel.de>)
Responses Re: Changing types of block and chunk sizes in memory contexts
List 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



pgsql-hackers by date:

Previous
From: Alena Rybakina
Date:
Subject: Re: POC, WIP: OR-clause support for indexes
Next
From: jian he
Date:
Subject: Re: Incremental View Maintenance, take 2