On Tue, Jul 02, 2013 at 10:33:38AM -0400, Noah Misch wrote:
> On Tue, Jul 02, 2013 at 12:08:42PM +0200, Andres Freund wrote:
> > On 2013-06-27 19:05:22 +0000, Noah Misch wrote:
> > > Permit super-MaxAllocSize allocations with MemoryContextAllocHuge().
> > >
> > > The MaxAllocSize guard is convenient for most callers, because it
> > > reduces the need for careful attention to overflow, data type selection,
> > > and the SET_VARSIZE() limit. A handful of callers are happy to navigate
> > > those hazards in exchange for the ability to allocate a larger chunk.
> > > Introduce MemoryContextAllocHuge() and repalloc_huge(). Use this in
> > > tuplesort.c and tuplestore.c, enabling internal sorts of up to INT_MAX
> > > tuples, a factor-of-48 increase. In particular, B-tree index builds can
> > > now benefit from much-larger maintenance_work_mem settings.
> >
> > This commit causes a bunch of warnings like:
> >
> > src/backend/utils/sort/tuplesort.c: In function
> > ???tuplesort_begin_common???:
> > src/backend/utils/sort/tuplesort.c:399:33: warning: comparison of unsigned
> > expression < 0 is always false
> > [-Wtype-limits]
> > #define LACKMEM(state) ((state)->availMem < 0)
> >
> > to be thrown during compilation. And I think it is spot on. Unless you
> > overhaul a good bit of the respective logic making availMem unsigned
> > isn't going to fly.
>
> True. Will look into it; thanks.
In retrospect, I did not need the "long" -> "Size" changes at all. I thought
they were necessary to prevent new overflow possibilities on 64-bit Windows,
but we already limited work_mem to 2 GiB on that platform to account for its
32-bit "long". That's a limit to be removed rather than perpetuated.
Therefore, rather than change back to "long", I've changed to "int64". That's
simple to reason about, optimal on 64-bit, and no big loss on 32-bit.
Thanks,
nm
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/backend/utils/sort/tuplesort.c
b/src/backend/utils/sort/tuplesort.c
index efc0760..7ca517b 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 ce1d476..be375a3 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 d5b3913..25fa6de 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'.
--
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
