Hi, Thanks for take a look. Em seg., 11 de jul. de 2022 às 05:48, Matthias van de Meent < boekewurm+postg...@gmail.com> escreveu:
> On Tue, 7 Jun 2022 at 03:09, Ranier Vilela <ranier...@gmail.com> wrote: > > > > Let's restart this, to simplify the review and commit work. > > The patchset fails to apply. Could you send an updated version that > applies to the current master branch? > Sure. > > > Regarding the patches now, we have: > > 1) v1-001-aset-reduces-memory-consumption.patch > > Reduces memory used by struct AllocBlockData by minus 8 bits, > > This seems reasonable, considering we don't generally use the field > for anything but validation. > > > reducing the total size to 32 bits, which leads to "fitting" two structs > in a 64bit cache. > > By bits, you mean bytes, right? > Correct. > > Regarding fitting 2 structs in 64 bytes, that point is moot, as each > of these structs are stored at the front of each malloc-ed block, so > you will never see more than one of these in the same cache line. Less > space used is nice, but not as critical there IMO. > > > Remove tests elog(ERROR, "could not find block containing chunk %p" and > > elog(ERROR, "could not find block containing chunk %p", moving them to > > MEMORY_CONTEXT_CHECKING context. > > > > Since 8.2 versions, nobody complains about these tests. > > > > But if is not acceptable, have the option (3) > v1-003-aset-reduces-memory-consumption.patch > > > > 2) v1-002-generation-reduces-memory-consumption.patch > > Reduces memory used by struct GenerationBlock, by minus 8 bits, > > That seems fairly straight-forward -- 8 bytes saved on each page isn't > a lot, but it's something. > > > reducing the total size to 32 bits, which leads to "fitting" two structs > in a 64bit cache. > > Your size accounting seems wrong. On 64-bit architectures, we have > dlist_node (=16) + Size (=8) + 2*int (=8) + 2 * (char*) (=16) = 48 > bytes. Shaving off the Size field reduces that by 8 bytes to 40 bytes. > > The argument of fitting 2 of these structures into one cache line is > moot again, because here, too, two of this struct will not share a > cache line (unless somehow we allocate 0-sized blocks, which would be > a bug). > Right. I think I was very tired. > > > 3) v1-003-aset-reduces-memory-consumption.patch > > Same to the (1), but without remove the tests: > > elog(ERROR, "could not find block containing chunk %p" and > > elog(ERROR, "could not find block containing chunk %p", > > But at the cost of removing a one tiny part of the tests and > > moving them to MEMORY_CONTEXT_CHECKING context. > > I like this patch over 001 due to allowing less corruption to occur in > the memory context code. This allows for detecting some issues in 003, > as opposed to none in 001. > I understand. regards, Ranier Vilela