On 6/28/23 12:59, Tom Lane wrote:
> David Rowley <dgrowleyml@gmail.com> writes:
>> Perhaps it's ok to leave the context creation functions with Size
>> typed parameters and then just Assert the passed-in sizes are not
>> larger than 1GB within the context creation function.
>
> Yes, I'm strongly opposed to not using Size/size_t in the mmgr APIs.
> If we go that road, we're going to have a problem when someone
> inevitably wants to pass a larger-than-GB value for some context
> type.
+1
> What happens in semi-private structs is a different matter, although
> I'm a little dubious that shaving a couple of bytes from context
> headers is a useful activity. The self-documentation argument
> still has some force there, so I agree with Peter that some positive
> benefit has to be shown.
>
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.
When I first saw the patch I was thinking it's probably changing how we
store the per-chunk requested_size. Maybe that'd make a difference,
although 4B is tiny compared to what we waste due to the doubling.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
