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