Horton

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

David Rowley <dgrowleyml@gmail.com>

View thread View on postgr.es
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-02T23:43:58Z
Lists: pgsql-hackers 
On Thu, 29 Sept 2022 at 18:30, David Rowley <dgrowleyml@gmail.com> wrote:
> Does anyone have any opinions on this?

I by no means think I've nailed the fix in
other_ideas_to_fix_MemoryContextContains.patch, so it would be good to
see if anyone else has any new ideas on how to solve this issue.

Andres did mention to me off-list about perhaps adding a boolean field
to FunctionCallInfoBaseData to indicate if the return value can be
assumed to be in CurrentMemoryContext.  I feel like that might be
quite a bit of work to go and change all functions to ensure that
that's properly populated. For example, look at split_part() in
varlena.c, it's going to be a little tedious to ensure we set that
field correctly there as that function sometimes returns it's input,
sometimes returns a string constant and sometimes allocates new
memory.  I feel fixing it this way will be error-prone and cause lots
of future bugs.

I'm also aware that the change made in b76fb6c2a becomes less
temporary with each day that passes, so I really would like to find a
solution to the MemoryContextContains issue. I'll reiterate that I
don't think reverting c6e0fe1f2 fixes MemoryContextContains. That
would just put back the behaviour of it returning true based on the
owning MemoryContext and/or the direction that the wind is coming from
on the particular day the function is called.

Although I do think other_ideas_to_fix_MemoryContextContains.patch
does fix the problem. I also fear a few people would be reaching for
their pitchforks if I was to go and commit it. However, as of now, I'm
starting to look more favourably at it as more time passes.

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