Alvaro Herrera wrote:
> Jinyu Zhang wrote:
>
> > Update the patch_brin_optimze_mem according to your comment. 
> 
> I have added this patch to the commitfest, which I've been intending to
> get in for a long time.  I'll be submitting an updated patch soon.

Here it is.  I addressed some of Tomas' comments, but not all (so this
is mostly just a rebased on Jinyu's original submission).

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 2c7963e..dc9cc2d 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -226,7 +226,8 @@ brin_page_items(PG_FUNCTION_ARGS)
                        if (ItemIdIsUsed(itemId))
                        {
                                dtup = brin_deform_tuple(bdesc,
-                                                                       
(BrinTuple *) PageGetItem(page, itemId));
+                                                                       
(BrinTuple *) PageGetItem(page, itemId),
+                                                                       NULL);
                                attno = 1;
                                unusedItem = false;
                        }
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index b22563b..04c50fe 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -182,7 +182,7 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
                        MemoryContextSwitchTo(tupcxt);
                }
 
-               dtup = brin_deform_tuple(bdesc, brtup);
+               dtup = brin_deform_tuple(bdesc, brtup, NULL);
 
                /*
                 * Compare the key values of the new tuple to the stored index 
values;
@@ -328,6 +328,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
        FmgrInfo   *consistentFn;
        MemoryContext oldcxt;
        MemoryContext perRangeCxt;
+       BrinMemTuple *dtup;
+       BrinTuple    *btup;
+       uint32       btupInitSize;
 
        opaque = (BrinOpaque *) scan->opaque;
        bdesc = opaque->bo_bdesc;
@@ -348,6 +351,13 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
         * key reference each of them.  We rely on zeroing fn_oid to InvalidOid.
         */
        consistentFn = palloc0(sizeof(FmgrInfo) * bdesc->bd_tupdesc->natts);
+       dtup = brin_new_memtuple(bdesc);
+
+       /*
+        * Estimate the approximate size of btup and allocate memory for btup.
+        */
+       btupInitSize = bdesc->bd_tupdesc->natts * 16;
+       btup = palloc(btupInitSize);
 
        /*
         * Setup and use a per-range memory context, which is reset every time 
we
@@ -379,7 +389,10 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
                                                                           
scan->xs_snapshot);
                if (tup)
                {
-                       tup = brin_copy_tuple(tup, size);
+                       if(size <= btupInitSize)
+                               memcpy(btup, tup, size);
+                       else
+                               btup = brin_copy_tuple(tup, size);
                        LockBuffer(buf, BUFFER_LOCK_UNLOCK);
                }
 
@@ -387,15 +400,13 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
                 * For page ranges with no indexed tuple, we must return the 
whole
                 * range; otherwise, compare it to the scan keys.
                 */
-               if (tup == NULL)
+               if (btup == NULL)
                {
                        addrange = true;
                }
                else
                {
-                       BrinMemTuple *dtup;
-
-                       dtup = brin_deform_tuple(bdesc, tup);
+                       dtup = brin_deform_tuple(bdesc, btup, dtup);
                        if (dtup->bt_placeholder)
                        {
                                /*
@@ -1175,7 +1186,7 @@ union_tuples(BrinDesc *bdesc, BrinMemTuple *a, BrinTuple 
*b)
                                                                "brin union",
                                                                
ALLOCSET_DEFAULT_SIZES);
        oldcxt = MemoryContextSwitchTo(cxt);
-       db = brin_deform_tuple(bdesc, b);
+       db = brin_deform_tuple(bdesc, b, NULL);
        MemoryContextSwitchTo(oldcxt);
 
        for (keyno = 0; keyno < bdesc->bd_tupdesc->natts; keyno++)
diff --git a/src/backend/access/brin/brin_tuple.c 
b/src/backend/access/brin/brin_tuple.c
index ec5fc56..20b0079 100644
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -348,54 +348,69 @@ BrinMemTuple *
 brin_new_memtuple(BrinDesc *brdesc)
 {
        BrinMemTuple *dtup;
-       char       *currdatum;
        long            basesize;
-       int                     i;
 
        basesize = MAXALIGN(sizeof(BrinMemTuple) +
                                                sizeof(BrinValues) * 
brdesc->bd_tupdesc->natts);
        dtup = palloc0(basesize + sizeof(Datum) * brdesc->bd_totalstored);
-       currdatum = (char *) dtup + basesize;
-       for (i = 0; i < brdesc->bd_tupdesc->natts; i++)
-       {
-               dtup->bt_columns[i].bv_attno = i + 1;
-               dtup->bt_columns[i].bv_allnulls = true;
-               dtup->bt_columns[i].bv_hasnulls = false;
-               dtup->bt_columns[i].bv_values = (Datum *) currdatum;
-               currdatum += sizeof(Datum) * brdesc->bd_info[i]->oi_nstored;
-       }
+
+       dtup->bt_values = palloc(sizeof(Datum) * brdesc->bd_totalstored);
+       dtup->bt_allnulls = palloc(sizeof(bool) * brdesc->bd_tupdesc->natts);
+       dtup->bt_hasnulls = palloc(sizeof(bool) * brdesc->bd_tupdesc->natts);
 
        dtup->bt_context = AllocSetContextCreate(CurrentMemoryContext,
                                                                                
         "brin dtuple",
                                                                                
         ALLOCSET_DEFAULT_SIZES);
+
+       brin_memtuple_initialize(dtup, brdesc);
+
        return dtup;
 }
 
 /*
- * Reset a BrinMemTuple to initial state
+ * Reset a BrinMemTuple to initial state.  We return the same tuple, for
+ * notational convenience.
  */
-void
+BrinMemTuple *
 brin_memtuple_initialize(BrinMemTuple *dtuple, BrinDesc *brdesc)
 {
        int                     i;
+       char       *currdatum;
 
        MemoryContextReset(dtuple->bt_context);
+
+       currdatum = (char *) dtuple +
+               MAXALIGN(sizeof(BrinMemTuple) +
+                                sizeof(BrinValues) * 
brdesc->bd_tupdesc->natts);
        for (i = 0; i < brdesc->bd_tupdesc->natts; i++)
        {
                dtuple->bt_columns[i].bv_allnulls = true;
                dtuple->bt_columns[i].bv_hasnulls = false;
+
+               dtuple->bt_columns[i].bv_attno = i + 1;
+               dtuple->bt_columns[i].bv_allnulls = true;
+               dtuple->bt_columns[i].bv_hasnulls = false;
+               dtuple->bt_columns[i].bv_values = (Datum *) currdatum;
+               currdatum += sizeof(Datum) * brdesc->bd_info[i]->oi_nstored;
        }
+
+       return dtuple;
 }
 
 /*
  * Convert a BrinTuple back to a BrinMemTuple.  This is the reverse of
  * brin_form_tuple.
  *
+ * As an optimization, the caller can pass a previously allocated 'dMemtuple'.
+ * This avoids having to allocate it here, which can be useful when this
+ * function is called many times in a loop.  It is caller's responsibility
+ * that the given BrinMemTuple matches what we need here.
+ *
  * Note we don't need the "on disk tupdesc" here; we rely on our own routine to
  * deconstruct the tuple from the on-disk format.
  */
 BrinMemTuple *
-brin_deform_tuple(BrinDesc *brdesc, BrinTuple *tuple)
+brin_deform_tuple(BrinDesc *brdesc, BrinTuple *tuple, BrinMemTuple *dMemtuple)
 {
        BrinMemTuple *dtup;
        Datum      *values;
@@ -407,15 +422,16 @@ brin_deform_tuple(BrinDesc *brdesc, BrinTuple *tuple)
        int                     valueno;
        MemoryContext oldcxt;
 
-       dtup = brin_new_memtuple(brdesc);
+       dtup = dMemtuple ? brin_memtuple_initialize(dMemtuple, brdesc) :
+               brin_new_memtuple(brdesc);
 
        if (BrinTupleIsPlaceholder(tuple))
                dtup->bt_placeholder = true;
        dtup->bt_blkno = tuple->bt_blkno;
 
-       values = palloc(sizeof(Datum) * brdesc->bd_totalstored);
-       allnulls = palloc(sizeof(bool) * brdesc->bd_tupdesc->natts);
-       hasnulls = palloc(sizeof(bool) * brdesc->bd_tupdesc->natts);
+       values = dtup->bt_values;
+       allnulls = dtup->bt_allnulls;
+       hasnulls = dtup->bt_hasnulls;
 
        tp = (char *) tuple + BrinTupleDataOffset(tuple);
 
@@ -458,10 +474,6 @@ brin_deform_tuple(BrinDesc *brdesc, BrinTuple *tuple)
 
        MemoryContextSwitchTo(oldcxt);
 
-       pfree(values);
-       pfree(allnulls);
-       pfree(hasnulls);
-
        return dtup;
 }
 
diff --git a/src/include/access/brin_tuple.h b/src/include/access/brin_tuple.h
index 6927865..86d8f98 100644
--- a/src/include/access/brin_tuple.h
+++ b/src/include/access/brin_tuple.h
@@ -38,6 +38,11 @@ typedef struct BrinMemTuple
        bool            bt_placeholder; /* this is a placeholder tuple */
        BlockNumber bt_blkno;           /* heap blkno that the tuple is for */
        MemoryContext bt_context;       /* memcxt holding the bt_columns values 
*/
+       /* output arrays for brin_deform_tuple: */
+       Datum           *bt_values;             /* values array */
+       bool            *bt_allnulls;   /* allnulls array */
+       bool            *bt_hasnulls;   /* hasnulls array */
+       /* not an output array, but must be last */
        BrinValues      bt_columns[FLEXIBLE_ARRAY_MEMBER];
 } BrinMemTuple;
 
@@ -88,9 +93,9 @@ extern bool brin_tuples_equal(const BrinTuple *a, Size alen,
                                  const BrinTuple *b, Size blen);
 
 extern BrinMemTuple *brin_new_memtuple(BrinDesc *brdesc);
-extern void brin_memtuple_initialize(BrinMemTuple *dtuple,
+extern BrinMemTuple *brin_memtuple_initialize(BrinMemTuple *dtuple,
                                                 BrinDesc *brdesc);
 extern BrinMemTuple *brin_deform_tuple(BrinDesc *brdesc,
-                                 BrinTuple *tuple);
+                                 BrinTuple *tuple, BrinMemTuple *dMemtuple);
 
 #endif   /* BRIN_TUPLE_H */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to