On Fri, May 27, 2022 at 10:52 AM Tom Lane <t...@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
