Re: tuplesort memory usage: grow_memtuples

On 8 December 2012 14:41, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> Is anybody planning to work on this? There hasn't been any activity
> since the beginning of the CF and it doesn't look like there is much
> work left?

I took another look at this.

The growmemtuples bool from Jeff's original patch has been re-added.
My strategy for preventing overflow is to use a uint64, and to use
Min()/Max() as appropriate. As Robert mentioned, even a 64-bit integer
could overflow here, and I account for that. Actually, right now this
is only a theoretical problem on 64-bit platforms, because of the
MaxAllocSize limitation - allowedMem being more than 2^38 (bytes, or
256GB) is a situation in which we won't repalloc anyway, because of
this:

/*
* On a 64-bit machine, allowedMem could be high enough to get us into
* trouble with MaxAllocSize, too.
*/
! if ((Size) (newmemtupsize) >= MaxAllocSize / sizeof(SortTuple))
! goto noalloc;

I reintroduced this check, absent in prior revisions, positioned
around the new code:

! /* We assume here that the memory chunk overhead associated with the
! * memtuples array is constant and so there will be no unexpected addition
! * to what we ask for. (The minimum array size established in
! * tuplesort_begin_common is large enough to force palloc to treat it as a
! * separate chunk, so this assumption should be good. But let's check it,
! * since the above fall-back may be used.)
*/
if (state->availMem <= (long) (state->memtupsize * sizeof(SortTuple)))
return false;

Though we use a uint64 for memtupsize here, we still don't fully trust
the final value:

! newmemtupsize = Min(Max(memtupsize * allowedMem / memNowUsed,
! memtupsize),
! memtupsize * 2);

I also added a brief note within tuplestore.c to the effect that the
two buffer sizing strategies are not in sync.

Thoughts?
--
Peter Geoghegan http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services