Re: Reducing the chunk header sizes on all memory context types

David Rowley <dgrowleyml@gmail.com>

From: David Rowley <dgrowleyml@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, Julien Rouhaud <rjuju123@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Yura Sokolov <y.sokolov@postgrespro.ru>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2022-10-06T20:44:49Z
Lists: pgsql-hackers
On Fri, 7 Oct 2022 at 09:05, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Here's a v2 incorporating discussed changes.
>
> In reordering enum MemoryContextMethodID, I arranged to avoid using
> 000 and 111 as valid IDs, since those bit patterns will appear in
> zeroed and wipe_mem'd memory respectively.  Those should probably be
> more-or-less-permanent exclusions, so I added comments about it.

I'm just considering some future developer here that is writing a new
MemoryContext type and there's no more space left and she or he needs
to either use 000 or 111. I think if that was me, I might be unsure if
I should be looking to expand the bit-space to make room. I might
think that based on the word "avoid" in:

> + MCTX_UNUSED1_ID, /* avoid: 000 occurs in never-used memory */
> + MCTX_UNUSED5_ID /* avoid: 111 occurs in wipe_mem'd memory */

but the final sentence in:

> + * dummy entries for unused IDs in the mcxt_methods[] array.  We also try
> + * to avoid using bit-patterns as valid IDs if they are likely to occur in
> + * garbage data.

leads me to believe we're just *trying* to avoid using these bit-patterns.

Also, the comment in mcxt_methods[] might make me believe that it's ok
for me to use them if I really need to.

> + * Unused (as yet) IDs should have dummy entries here.  This allows us to

Based on these comments, I'm not quite sure if I should be completely
avoiding using 000 and 111 or I should just use those last when there
are no other free slots in the array. It might be quite a long time
before someone is in this situation, so should we be more clear?

However, maybe you've left it this way as you feel it's a decision
that must be made in the future, perhaps based on how difficult it
would be to free up another bit?

David



Commits

  1. Harden memory context allocators against bogus chunk pointers.

  2. Improve our ability to detect bogus pointers passed to pfree et al.

  3. Remove MemoryContextContains().

  4. Remove uses of MemoryContextContains in nodeAgg.c and nodeWindowAgg.c.

  5. Temporarily make MemoryContextContains return false

  6. Make MemoryContextContains work correctly again

  7. Make more effort to put a sentinel at the end of allocated memory

  8. Fix some possibly latent bugs in slab.c

  9. Various cleanups of the new memory context header code

  10. Revert "Add missing padding from MemoryChunk struct"

  11. Use MAXALIGN() in calculations using sizeof(SlabBlock)

  12. Add missing padding from MemoryChunk struct

  13. Improve performance of and reduce overheads of memory management