Horton

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

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <dgrowleyml@gmail.com>
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-04T15:55:06Z
Lists: pgsql-hackers

Attachments

I wrote:
> I think what we should look at is extending the aggregate/window
> function APIs so that such functions can report where they put their
> output, and then we can nuke MemoryContextContains(), with the
> code code set up to assume that it has to copy if the called function
> didn't report anything.  The existing FunctionCallInfo.resultinfo
> mechanism (cf. ReturnSetInfo for SRFs) is probably the right thing
> to pass the flag through.

After studying the existing usages of MemoryContextContains, I think
there is a better answer, which is to just nuke them.

As far as I can tell, the datumCopy steps associated with aggregate
finalfns are basically useless.  They only serve to prevent
returning a pointer directly to the aggregate's transition value
(or, perhaps, to a portion thereof).  But what's wrong with that?
It'll last as long as we need it to.  Maybe there was a reason
back before we required finalfns to not scribble on the transition
values, but those days are gone.

The same goes for aggregate serialfns --- although there, I can't
avoid the feeling that the datumCopy step was just cargo-culted in.
I don't think there can exist a serialfn that doesn't return a
freshly-palloced bytea.

The one place where we actually need the conditional datumCopy is
with window functions, and even there I don't think we need it
in simple cases with only one window function.  The case that is
hazardous is where multiple window functions are sharing a
WindowObject.  So I'm content to optimize the single-window-function
case and just always copy if there's more than one.  (Sadly, there
is no existing regression test that catches this, so I added one.)

See attached.

			regards, tom lane

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