Re: tuplesort memory usage: grow_memtuples

On 8 December 2012 14:41, Andres Freund <andres@2ndquadrant.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