On 07/13/2018 09:44 AM, Heikki Linnakangas wrote:
On 13/07/18 01:39, Andrew Dunstan wrote:
On 07/12/2018 06:34 PM, Alvaro Herrera wrote:
On 2018-Jul-12, Andrew Dunstan wrote:

I fully understand. I think this needs to go back to "Waiting on Author".
Why?  Heikki's patch applies fine and passes the regression tests.

Well, I understood Claudio was going to do some more work (see
upthread).

Claudio raised a good point, that doing small pallocs leads to fragmentation, and in particular, it might mean that we can't give back the memory to the OS. The default glibc malloc() implementation has a threshold of 4 or 32 MB or something like that - allocations larger than the threshold are mmap()'d, and can always be returned to the OS. I think a simple solution to that is to allocate larger chunks, something like 32-64 MB at a time, and carve out the allocations for the nodes from those chunks. That's pretty straightforward, because we don't need to worry about freeing the nodes in retail. Keep track of the current half-filled chunk, and allocate a new one when it fills up.


Google seems to suggest the default threshold is much lower, like 128K. Still, making larger allocations seems sensible. Are you going to work on that?



He also wanted to refactor the iterator API, to return one ItemPointer at a time. I don't think that's necessary, the current iterator API is more convenient for the callers, but I don't feel strongly about that.

Anything else?

If we're going to go with Heikki's patch then do we need to
change the author, or add him as an author?

Let's list both of us. At least in the commit message, doesn't matter much what the commitfest app says.



I added you as an author in the CF App

cheers

andrew

--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply via email to