Move memory management away from writetup() and tuplesort_put*()

This commit puts some generic work away from sort-variant-specific function.
In particular, tuplesort_put*() now doesn't need to decrease available memory
and switch to sort context before calling puttuple_common().  writetup()
doesn't need to free SortTuple.tuple and increase available memory.

Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent
Reviewed-by: Andres Freund, John Naylor

diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 828efe701e55c1070d60a6d50f1f2ddff6e65a81..c8c511fb8c5246a96593bfa3b469b9018a14f04c 100644 (file)
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -288,11 +288,7 @@ struct Tuplesortstate
 
    /*
     * Function to write a stored tuple onto tape.  The representation of the
-    * tuple on tape need not be the same as it is in memory; requirements on
-    * the tape representation are given below.  Unless the slab allocator is
-    * used, after writing the tuple, pfree() the out-of-line data (not the
-    * SortTuple struct!), and increase state->availMem by the amount of
-    * memory space thereby released.
+    * tuple on tape need not be the same as it is in memory.
     */
    void        (*writetup) (Tuplesortstate *state, LogicalTape *tape,
                             SortTuple *stup);
@@ -549,7 +545,7 @@ struct Sharedsort
 
 #define REMOVEABBREV(state,stup,count) ((*(state)->removeabbrev) (state, stup, count))
 #define COMPARETUP(state,a,b)  ((*(state)->comparetup) (a, b, state))
-#define WRITETUP(state,tape,stup)  ((*(state)->writetup) (state, tape, stup))
+#define WRITETUP(state,tape,stup)  (writetuple(state, tape, stup))
 #define READTUP(state,stup,tape,len) ((*(state)->readtup) (state, stup, tape, len))
 #define LACKMEM(state)     ((state)->availMem < 0 && !(state)->slabAllocatorUsed)
 #define USEMEM(state,amt)  ((state)->availMem -= (amt))
@@ -618,6 +614,8 @@ static Tuplesortstate *tuplesort_begin_common(int workMem,
 static void tuplesort_begin_batch(Tuplesortstate *state);
 static void puttuple_common(Tuplesortstate *state, SortTuple *tuple,
                            bool useAbbrev);
+static void writetuple(Tuplesortstate *state, LogicalTape *tape,
+                      SortTuple *stup);
 static bool consider_abort_common(Tuplesortstate *state);
 static void inittapes(Tuplesortstate *state, bool mergeruns);
 static void inittapestate(Tuplesortstate *state, int maxTapes);
@@ -1848,7 +1846,6 @@ tuplesort_puttupleslot(Tuplesortstate *state, TupleTableSlot *slot)
    /* copy the tuple into sort storage */
    tuple = ExecCopySlotMinimalTuple(slot);
    stup.tuple = (void *) tuple;
-   USEMEM(state, GetMemoryChunkSpace(tuple));
    /* set up first-column key value */
    htup.t_len = tuple->t_len + MINIMAL_TUPLE_OFFSET;
    htup.t_data = (HeapTupleHeader) ((char *) tuple - MINIMAL_TUPLE_OFFSET);
@@ -1857,8 +1854,6 @@ tuplesort_puttupleslot(Tuplesortstate *state, TupleTableSlot *slot)
                               state->tupDesc,
                               &stup.isnull1);
 
-   MemoryContextSwitchTo(state->sortcontext);
-
    puttuple_common(state, &stup,
                    state->sortKeys->abbrev_converter && !stup.isnull1);
 
@@ -1879,9 +1874,6 @@ tuplesort_putheaptuple(Tuplesortstate *state, HeapTuple tup)
    /* copy the tuple into sort storage */
    tup = heap_copytuple(tup);
    stup.tuple = (void *) tup;
-   USEMEM(state, GetMemoryChunkSpace(tup));
-
-   MemoryContextSwitchTo(state->sortcontext);
 
    /*
     * set up first-column key value, and potentially abbreviate, if it's a
@@ -1910,7 +1902,6 @@ tuplesort_putindextuplevalues(Tuplesortstate *state, Relation rel,
                              ItemPointer self, Datum *values,
                              bool *isnull)
 {
-   MemoryContext oldcontext;
    SortTuple   stup;
    IndexTuple  tuple;
 
@@ -1918,19 +1909,14 @@ tuplesort_putindextuplevalues(Tuplesortstate *state, Relation rel,
                                          isnull, state->tuplecontext);
    tuple = ((IndexTuple) stup.tuple);
    tuple->t_tid = *self;
-   USEMEM(state, GetMemoryChunkSpace(stup.tuple));
    /* set up first-column key value */
    stup.datum1 = index_getattr(tuple,
                                1,
                                RelationGetDescr(state->indexRel),
                                &stup.isnull1);
 
-   oldcontext = MemoryContextSwitchTo(state->sortcontext);
-
    puttuple_common(state, &stup,
                    state->sortKeys && state->sortKeys->abbrev_converter && !stup.isnull1);
-
-   MemoryContextSwitchTo(oldcontext);
 }
 
 /*
@@ -1965,15 +1951,12 @@ tuplesort_putdatum(Tuplesortstate *state, Datum val, bool isNull)
        stup.datum1 = !isNull ? val : (Datum) 0;
        stup.isnull1 = isNull;
        stup.tuple = NULL;      /* no separate storage */
-       MemoryContextSwitchTo(state->sortcontext);
    }
    else
    {
        stup.isnull1 = false;
        stup.datum1 = datumCopy(val, false, state->datumTypeLen);
        stup.tuple = DatumGetPointer(stup.datum1);
-       USEMEM(state, GetMemoryChunkSpace(stup.tuple));
-       MemoryContextSwitchTo(state->sortcontext);
    }
 
    puttuple_common(state, &stup,
@@ -1988,8 +1971,14 @@ tuplesort_putdatum(Tuplesortstate *state, Datum val, bool isNull)
 static void
 puttuple_common(Tuplesortstate *state, SortTuple *tuple, bool useAbbrev)
 {
+   MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
+
    Assert(!LEADER(state));
 
+   /* Count the size of the out-of-line data */
+   if (tuple->tuple != NULL)
+       USEMEM(state, GetMemoryChunkSpace(tuple->tuple));
+
    if (!useAbbrev)
    {
        /*
@@ -2062,6 +2051,7 @@ puttuple_common(Tuplesortstate *state, SortTuple *tuple, bool useAbbrev)
                         pg_rusage_show(&state->ru_start));
 #endif
                make_bounded_heap(state);
+               MemoryContextSwitchTo(oldcontext);
                return;
            }
 
@@ -2069,7 +2059,10 @@ puttuple_common(Tuplesortstate *state, SortTuple *tuple, bool useAbbrev)
             * Done if we still fit in available memory and have array slots.
             */
            if (state->memtupcount < state->memtupsize && !LACKMEM(state))
+           {
+               MemoryContextSwitchTo(oldcontext);
                return;
+           }
 
            /*
             * Nope; time to switch to tape-based operation.
@@ -2123,6 +2116,25 @@ puttuple_common(Tuplesortstate *state, SortTuple *tuple, bool useAbbrev)
            elog(ERROR, "invalid tuplesort state");
            break;
    }
+   MemoryContextSwitchTo(oldcontext);
+}
+
+/*
+ * Write a stored tuple onto tape.tuple.  Unless the slab allocator is
+ * used, after writing the tuple, pfree() the out-of-line data (not the
+ * SortTuple struct!), and increase state->availMem by the amount of
+ * memory space thereby released.
+ */
+static void
+writetuple(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
+{
+   state->writetup(state, tape, stup);
+
+   if (!state->slabAllocatorUsed && stup->tuple)
+   {
+       FREEMEM(state, GetMemoryChunkSpace(stup->tuple));
+       pfree(stup->tuple);
+   }
 }
 
 static bool
@@ -3960,12 +3972,6 @@ writetup_heap(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
    if (state->sortopt & TUPLESORT_RANDOMACCESS)    /* need trailing length
                                                     * word? */
        LogicalTapeWrite(tape, (void *) &tuplen, sizeof(tuplen));
-
-   if (!state->slabAllocatorUsed)
-   {
-       FREEMEM(state, GetMemoryChunkSpace(tuple));
-       heap_free_minimal_tuple(tuple);
-   }
 }
 
 static void
@@ -4141,12 +4147,6 @@ writetup_cluster(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
    if (state->sortopt & TUPLESORT_RANDOMACCESS)    /* need trailing length
                                                     * word? */
        LogicalTapeWrite(tape, &tuplen, sizeof(tuplen));
-
-   if (!state->slabAllocatorUsed)
-   {
-       FREEMEM(state, GetMemoryChunkSpace(tuple));
-       heap_freetuple(tuple);
-   }
 }
 
 static void
@@ -4403,12 +4403,6 @@ writetup_index(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
    if (state->sortopt & TUPLESORT_RANDOMACCESS)    /* need trailing length
                                                     * word? */
        LogicalTapeWrite(tape, (void *) &tuplen, sizeof(tuplen));
-
-   if (!state->slabAllocatorUsed)
-   {
-       FREEMEM(state, GetMemoryChunkSpace(tuple));
-       pfree(tuple);
-   }
 }
 
 static void
@@ -4495,12 +4489,6 @@ writetup_datum(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
    if (state->sortopt & TUPLESORT_RANDOMACCESS)    /* need trailing length
                                                     * word? */
        LogicalTapeWrite(tape, (void *) &writtenlen, sizeof(writtenlen));
-
-   if (!state->slabAllocatorUsed && stup->tuple)
-   {
-       FREEMEM(state, GetMemoryChunkSpace(stup->tuple));
-       pfree(stup->tuple);
-   }
 }
 
 static void