From 79e0f87a15643efa9a94e011da509746dbb96798 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Thu, 4 Jul 2013 23:09:54 -0400 Subject: [PATCH] Use type "int64" for memory accounting in tuplesort.c/tuplestore.c. Commit 263865a48973767ce8ed7b7788059a38a24a9f37 switched tuplesort.c and tuplestore.c variables representing memory usage from type "long" to type "Size". This was unnecessary; I thought doing so avoided overflow scenarios on 64-bit Windows, but guc.c already limited work_mem so as to prevent the overflow. It was also incomplete, not touching the logic that assumed a signed data type. Change the affected variables to "int64". This is perfect for 64-bit platforms, and it reduces the need to contemplate platform-specific overflow scenarios. It also puts us close to being able to support work_mem over 2 GiB on 64-bit Windows. Per report from Andres Freund. --- src/backend/utils/sort/tuplesort.c | 30 +++++++++++++++-------------- src/backend/utils/sort/tuplestore.c | 14 ++++++++------ src/include/utils/tuplesort.h | 2 +- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index efc0760640..7ca517b357 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -211,8 +211,8 @@ struct Tuplesortstate * tuples to return? */ bool boundUsed; /* true if we made use of a bounded heap */ int bound; /* if bounded, the maximum number of tuples */ - Size availMem; /* remaining memory available, in bytes */ - Size allowedMem; /* total memory allowed, in bytes */ + int64 availMem; /* remaining memory available, in bytes */ + int64 allowedMem; /* total memory allowed, in bytes */ int maxTapes; /* number of tapes (Knuth's T) */ int tapeRange; /* maxTapes-1 (Knuth's P) */ MemoryContext sortcontext; /* memory context holding all sort data */ @@ -308,7 +308,7 @@ struct Tuplesortstate int *mergenext; /* first preread tuple for each source */ int *mergelast; /* last preread tuple for each source */ int *mergeavailslots; /* slots left for prereading each tape */ - Size *mergeavailmem; /* availMem for prereading each tape */ + int64 *mergeavailmem; /* availMem for prereading each tape */ int mergefreelist; /* head of freelist of recycled slots */ int mergefirstfree; /* first slot never used in this merge */ @@ -565,7 +565,7 @@ tuplesort_begin_common(int workMem, bool randomAccess) state->randomAccess = randomAccess; state->bounded = false; state->boundUsed = false; - state->allowedMem = workMem * 1024L; + state->allowedMem = workMem * (int64) 1024; state->availMem = state->allowedMem; state->sortcontext = sortcontext; state->tapeset = NULL; @@ -980,7 +980,7 @@ grow_memtuples(Tuplesortstate *state) { int newmemtupsize; int memtupsize = state->memtupsize; - Size memNowUsed = state->allowedMem - state->availMem; + int64 memNowUsed = state->allowedMem - state->availMem; /* Forget it if we've already maxed out memtuples, per comment above */ if (!state->growmemtuples) @@ -991,7 +991,7 @@ grow_memtuples(Tuplesortstate *state) { /* * We've used no more than half of allowedMem; double our usage, - * clamping at INT_MAX. + * clamping at INT_MAX tuples. */ if (memtupsize < INT_MAX / 2) newmemtupsize = memtupsize * 2; @@ -1048,7 +1048,9 @@ grow_memtuples(Tuplesortstate *state) /* * On a 32-bit machine, allowedMem could exceed MaxAllocHugeSize. Clamp * to ensure our request won't be rejected. Note that we can easily - * exhaust address space before facing this outcome. + * exhaust address space before facing this outcome. (This is presently + * impossible due to guc.c's MAX_KILOBYTES limitation on work_mem, but + * don't rely on that at this distance.) */ if ((Size) newmemtupsize >= MaxAllocHugeSize / sizeof(SortTuple)) { @@ -1067,7 +1069,7 @@ grow_memtuples(Tuplesortstate *state) * palloc would be treating both old and new arrays as separate chunks. * But we'll check LACKMEM explicitly below just in case.) */ - if (state->availMem < (Size) ((newmemtupsize - memtupsize) * sizeof(SortTuple))) + if (state->availMem < (int64) ((newmemtupsize - memtupsize) * sizeof(SortTuple))) goto noalloc; /* OK, do it */ @@ -1722,7 +1724,7 @@ tuplesort_getdatum(Tuplesortstate *state, bool forward, * This is exported for use by the planner. allowedMem is in bytes. */ int -tuplesort_merge_order(Size allowedMem) +tuplesort_merge_order(int64 allowedMem) { int mOrder; @@ -1756,7 +1758,7 @@ inittapes(Tuplesortstate *state) int maxTapes, ntuples, j; - Size tapeSpace; + int64 tapeSpace; /* Compute number of tapes to use: merge order plus 1 */ maxTapes = tuplesort_merge_order(state->allowedMem) + 1; @@ -1805,7 +1807,7 @@ inittapes(Tuplesortstate *state) state->mergenext = (int *) palloc0(maxTapes * sizeof(int)); state->mergelast = (int *) palloc0(maxTapes * sizeof(int)); state->mergeavailslots = (int *) palloc0(maxTapes * sizeof(int)); - state->mergeavailmem = (Size *) palloc0(maxTapes * sizeof(Size)); + state->mergeavailmem = (int64 *) palloc0(maxTapes * sizeof(int64)); state->tp_fib = (int *) palloc0(maxTapes * sizeof(int)); state->tp_runs = (int *) palloc0(maxTapes * sizeof(int)); state->tp_dummy = (int *) palloc0(maxTapes * sizeof(int)); @@ -2033,7 +2035,7 @@ mergeonerun(Tuplesortstate *state) int srcTape; int tupIndex; SortTuple *tup; - Size priorAvail, + int64 priorAvail, spaceFreed; /* @@ -2107,7 +2109,7 @@ beginmerge(Tuplesortstate *state) int tapenum; int srcTape; int slotsPerTape; - Size spacePerTape; + int64 spacePerTape; /* Heap should be empty here */ Assert(state->memtupcount == 0); @@ -2228,7 +2230,7 @@ mergeprereadone(Tuplesortstate *state, int srcTape) unsigned int tuplen; SortTuple stup; int tupIndex; - Size priorAvail, + int64 priorAvail, spaceUsed; if (!state->mergeactive[srcTape]) diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c index ce1d47611c..be375a362c 100644 --- a/src/backend/utils/sort/tuplestore.c +++ b/src/backend/utils/sort/tuplestore.c @@ -104,8 +104,8 @@ struct Tuplestorestate bool backward; /* store extra length words in file? */ bool interXact; /* keep open through transactions? */ bool truncated; /* tuplestore_trim has removed tuples? */ - Size availMem; /* remaining memory available, in bytes */ - Size allowedMem; /* total memory allowed, in bytes */ + int64 availMem; /* remaining memory available, in bytes */ + int64 allowedMem; /* total memory allowed, in bytes */ BufFile *myfile; /* underlying file, or NULL if none */ MemoryContext context; /* memory context for holding tuples */ ResourceOwner resowner; /* resowner for holding temp files */ @@ -550,7 +550,7 @@ grow_memtuples(Tuplestorestate *state) { int newmemtupsize; int memtupsize = state->memtupsize; - Size memNowUsed = state->allowedMem - state->availMem; + int64 memNowUsed = state->allowedMem - state->availMem; /* Forget it if we've already maxed out memtuples, per comment above */ if (!state->growmemtuples) @@ -561,7 +561,7 @@ grow_memtuples(Tuplestorestate *state) { /* * We've used no more than half of allowedMem; double our usage, - * clamping at INT_MAX. + * clamping at INT_MAX tuples. */ if (memtupsize < INT_MAX / 2) newmemtupsize = memtupsize * 2; @@ -618,7 +618,9 @@ grow_memtuples(Tuplestorestate *state) /* * On a 32-bit machine, allowedMem could exceed MaxAllocHugeSize. Clamp * to ensure our request won't be rejected. Note that we can easily - * exhaust address space before facing this outcome. + * exhaust address space before facing this outcome. (This is presently + * impossible due to guc.c's MAX_KILOBYTES limitation on work_mem, but + * don't rely on that at this distance.) */ if ((Size) newmemtupsize >= MaxAllocHugeSize / sizeof(void *)) { @@ -637,7 +639,7 @@ grow_memtuples(Tuplestorestate *state) * palloc would be treating both old and new arrays as separate chunks. * But we'll check LACKMEM explicitly below just in case.) */ - if (state->availMem < (Size) ((newmemtupsize - memtupsize) * sizeof(void *))) + if (state->availMem < (int64) ((newmemtupsize - memtupsize) * sizeof(void *))) goto noalloc; /* OK, do it */ diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h index d5b391367e..25fa6de18e 100644 --- a/src/include/utils/tuplesort.h +++ b/src/include/utils/tuplesort.h @@ -106,7 +106,7 @@ extern void tuplesort_get_stats(Tuplesortstate *state, const char **spaceType, long *spaceUsed); -extern int tuplesort_merge_order(Size allowedMem); +extern int tuplesort_merge_order(int64 allowedMem); /* * These routines may only be called if randomAccess was specified 'true'.