Re: Changing types of block and chunk sizes in memory contexts
От | David Rowley |
---|---|
Тема | Re: Changing types of block and chunk sizes in memory contexts |
Дата | |
Msg-id | CAApHDvr+QF9CeSCVpZN0GyyvoGwG2-hSHaZ=VYa98vzzmuM_cw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Changing types of block and chunk sizes in memory contexts (Melih Mutlu <m.melihmutlu@gmail.com>) |
Ответы |
Re: Changing types of block and chunk sizes in memory contexts
|
Список | pgsql-hackers |
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
Вложения
В списке pgsql-hackers по дате отправления: