Re: Add bump memory context type and use it for tuplesorts

On Sat, 6 Apr 2024, 14:36 David Rowley, <dgrowleyml@gmail.com> wrote:

> On Sat, 6 Apr 2024 at 02:30, Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
> >
> > On Thu, 4 Apr 2024 at 22:43, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >
> > > Matthias van de Meent <boekewurm+postgres@gmail.com> writes:
> > > > It extends memory context IDs to 5 bits (32 values), of which
> > > > - 8 have glibc's malloc pattern of 001/010;
> > > > - 1 is unused memory's 00000
> > > > - 1 is wipe_mem's 11111
> > > > - 4 are used by existing contexts
> (Aset/Generation/Slab/AlignedRedirect)
> > > > - 18 are newly available.
> > >
> > > This seems like it would solve the problem for a good long time
> > > to come; and if we ever need more IDs, we could steal one more bit
> > > by requiring the offset to the block header to be a multiple of 8.
> > > (Really, we could just about do that today at little or no cost ...
> > > machines with MAXALIGN less than 8 are very thin on the ground.)
> >
> > Hmm, it seems like a decent idea, but I didn't want to deal with the
> > repercussions of that this late in the cycle when these 2 bits were
> > still relatively easy to get hold of.
>
> Thanks for writing the patch.
>
> I think 5 bits is 1 too many. 4 seems fine. I also think you've
> reserved too many slots in your patch as I disagree that we need to
> reserve the glibc malloc pattern anywhere but in the 1 and 2 slots of
> the mcxt_methods[] array.  I looked again at the 8 bytes prior to a
> glibc malloc'd chunk and I see the lowest 4 bits of the headers
> consistently set to 0001 for all powers of 2 starting at 8 up to
> 65536.


Malloc's docs specify the minimum chunk size at 4*sizeof(void*) and itself
uses , so using powers of 2 for chunks would indeed fail to detect 1s in
the 4th bit. I suspect you'll get different results when you check the
allocation patterns of multiples of 8 bytes, starting from 40, especially
on 32-bit arm (where MALLOC_ALIGNMENT is 8 bytes, rather than the 16 bytes
on i386 and 64-bit architectures, assuming  [0] is accurate)

131072 seems to vary and beyond that, they seem to be set to
> 0010.
>

In your updated 0001, you don't seem to fill the RESERVED_GLIBC memctx
array entries with BOGUS_MCTX().

With that, there's no increase in the number of reserved slots from
> what we have reserved today. Still 4.  So having 4 bits instead of 3
> bits gives us a total of 12 slots rather than 4 slots.  Having 3x
> slots seems enough.  We might need an extra bit for something else
> sometime. I think keeping it up our sleeve is a good idea.
>
> Another reason not to make it 5 bits is that I believe that would make
> the mcxt_methods[] array 2304 bytes rather than 576 bytes.  4 bits
> makes it 1152 bytes, if I'm counting correctly.
>

I don't think I understand why this would be relevant when only 5 of the
contexts are actually in use (thus in caches). Is that size concern about
TLB entries then?


> I revised the patch to simplify hdrmask logic.  This started with me
> having trouble finding the best set of words to document that the
> offset is "half the bytes between the chunk and block".  So, instead
> of doing that, I've just made it so these two fields effectively
> overlap. The lowest bit of the block offset is the same bit as the
> high bit of what MemoryChunkGetValue returns.


Works for me, I suppose.

I also updated src/backend/utils/mmgr/README to explain this and
> adjust the mentions of 3-bits and 61-bits to 4-bits and 60-bits.  I
> also explained the overlapping part.
>

Thanks!

[0]
https://sourceware.org/glibc/wiki/MallocInternals#Platform-specific_Thresholds_and_Constants

>