On Tue, 11 Jul 2023 at 02:41, Melih Mutlu <m.melihmutlu@gmail.com> wrote:
> > - Afaics AllocSet->keeper is unnecessary these days, as it is always allocated
> > together with the context itself. Saves 8 bytes.
>
> This seemed like a safe change and removed the keeper field in
> AllocSet and Generation contexts. It saves an additional 8 bytes.
Seems like a good idea for an additional 8-bytes.
I looked at your v2 patch. The only thing that really looked wrong
were the (Size) casts in the context creation functions. These should
have been casts to uint32 rather than Size. Basically, all the casts
do is say to the compiler "Yes, I know this could cause truncation due
to assigning to a size smaller than the source type's size". Some
compilers will likely warn without that and the cast will stop them.
We know there can't be any truncation due to the Asserts. There's also
the fundamental limitation that MemoryChunk can't store block offsets
larger than 1GBs anyway, so things will go bad if we tried to have
blocks bigger than 1GB.
Aside from that, I thought that a couple of other slab.c fields could
be shrunken to uint32 as the v2 patch just reduces the size of 1 field
which just creates a 4-byte hole in SlabContext. The fullChunkSize
field is just the MAXALIGN(chunkSize) + sizeof(MemoryChunk). We
should never be using slab contexts for any chunks anywhere near that
size. aset.c would be a better context for that, so it seems fine to
me to further restrict the maximum supported chunk size by another 8
bytes.
I've attached your patch again along with a small delta of what I adjusted.
My thoughts on these changes are that it's senseless to have Size
typed fields for storing a value that's never larger than 2^30.
Getting rid of the keeper pointer seems like a cleanup as it's pretty
much a redundant field. For small sized contexts like the ones used
for storing index relcache entries, I think it makes sense to save 20
more bytes. Each backend can have many thousand of those and there
could be many hundred backends. If we can fit more allocations on that
initial 1 kilobyte keeper block without having to allocate any
additional blocks, then that's great.
I feel that Andres's results showing several hundred fewer block
allocations shows this working. Albeit, his patch reduced the size of
the structs even further than what v3 does. I think v3 is enough for
now as the additional changes Andres mentioned require some more
invasive code changes to make work.
If nobody objects or has other ideas about this, modulo commit
message, I plan to push the attached on Monday.
David