On 6/28/23 12:59, Tom Lane wrote: > David Rowley <dgrowle...@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
