On Fri, May 27, 2022 at 10:52 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Given David's results in the preceding message, I don't think I am.
> A scheme like this would add more arithmetic and at least one more
> indirection to GetMemoryChunkContext(), and we already know that
> adding even a test-and-branch there has measurable cost.
I think you're being too negative here. It's a 7% regression on 8-byte
allocations in a tight loop. In real life, people allocate memory
because they want to do something with it, and the allocation overhead
therefore figures to be substantially less. They also nearly always
allocate memory more than 8 bytes at a time, since there's very little
of interest that can fit into an 8 byte allocation, and if you're
dealing with one of the things that can, you're likely to allocate an
array rather than each item individually. I think it's quite plausible
that saving space is going to be more important for performance than
the tiny cost of a test-and-branch here.
I don't want to take the position that we ought necessarily to commit
your patch, because I don't really have a clear sense of what the wins
and losses actually are. But, I am worried that our whole memory
allocation infrastructure is stuck at a local maximum, and I think
your patch pushes in a generally healthy direction: let's optimize for
wasting less space, instead of for the absolute minimum number of CPU
cycles consumed.
aset.c's approach is almost unbeatable for small numbers of
allocations in tiny contexts, and PostgreSQL does a lot of that. But
when you do have cases where a lot of data needs to be stored in
memory, it starts to look pretty lame. To really get out from under
that problem, we'd need to find a way to remove the requirement of a
per-allocation header altogether, and I don't think this patch really
helps us see how we could ever get all the way to that point.
Nonetheless, I like the fact that it puts more flexibility into the
mechanism seemingly at very little real cost, and that it seems to
mean less memory spent on header information rather than user data.
--
Robert Haas
EDB: http://www.enterprisedb.com
