Re: Reducing Memory Consumption (aset and generation)

Hi,
Thanks for take a look.

Em seg., 11 de jul. de 2022 às 05:48, Matthias van de Meent <
boekewurm+postgres(at)gmail(dot)com> escreveu:

> On Tue, 7 Jun 2022 at 03:09, Ranier Vilela <ranier(dot)vf(at)gmail(dot)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