List:Commits« Previous MessageNext Message »
From:Ole John Aske Date:November 8 2012 12:05pm
Subject:bzr push into mysql-5.1-telco-7.0 branch (ole.john.aske:5028 to 5029)
Bug#14709490
View as plain text  
 5029 Ole John Aske	2012-11-08
      SPJ refactoring on preparation for fixing bug#14709490.
      
      This is mainly a refactorication of the RowBuffer utility methods, and
      the Map and List collection used to organize buffered rows.
      
      This patch is an aggregate of the patches p1 - p10 described below:
      (Reviewed by Jan Wedvik)
      
      ===========
      
      p1.patch
        Remove unused member field 'm_src_node_no' from struct RowPtr.
      
      p2.patch:
        Refactor DEBUG #defines in SPJ block.
      
      p3.patch
        Fix / refactor of 'struct RowBuffer':
        The RowBuffer could either be organized as 'stack or 'var' allocated rows.
        However the RowBuffer c'tor, and the reset of it in
        Dbspj::releaseRequestBuffers() hardcoded the assumption that it always
        was a stack. This was incorrect as a 'multi-scan' would request
        a var allocated RowBuffer. Added 'type' info & arguments to
        RowBuffer to fix this.
      
      p4.patch
       Introduce Dbspj::rowAlloc(RowBuffer&...) which allocate buffer
       memory for a row either as 'stack' or 'var' buffer memory depending
       on the type of the specified RowBuffer.
      
      p5.patch
        Remove unused SLFifoRowListIteratorPtr & RowMapIteratorPtr and the
        ::next iterators using this type
      
      p6.patch
        Deprecate Request::RT_VAR_ALLOC which was used to check if a 'var' or
        'stack' allocated RowBuffer was being used. Instead use the 'type'
        of the RowBuffer itself.
      
      p7.patch
        Replace get_row_ptr_stack() & get_row_ptr_var() with the common
        function get_row_ptr().  The requested  memory will then be allocated
        as either 'stack' or 'var' depending on the supplied 'RowRef.m_allocator'
        argument.
      
      p8.patch
       Introduce 'struct RowCollection' and 'struct RowIterator' as an
       abstraction on top of the existing 'RowMap' and 'SLFifoRowList'
       collection and iterator classes.
       Also introduce new first() and next() iterators which takes
       advantage of the new abstractions.
      
      p9.patch
       Change interface to several Dbspj methods such that they now take the
       RowCollection as an argument instead of being hardcoded that it
       should always use Request::m_rows. This enables future use of
       multiple RowCollections where appropriate.
      
      p10.patch
       Refactor RowBuffer setup into own method Dbspj::initRowBuffers()

    modified:
      storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp
      storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp
 5028 magnus.blaudd@stripped	2012-11-07 [merge]
      Merge

    modified:
      client/mysqldump.c
      storage/ndb/include/mgmapi/mgmapi.h
=== modified file 'storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp'

=== modified file 'storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp'
--- a/storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp	2012-09-19 06:37:24 +0000
+++ b/storage/ndb/src/kernel/blocks/dbspj/Dbspj.hpp	2012-11-08 11:55:49 +0000
@@ -152,6 +152,12 @@
   };
   typedef Ptr<TableRecord> TableRecordPtr;
 
+  enum Buffer_type {
+    BUFFER_VOID  = 0,
+    BUFFER_STACK = 1,
+    BUFFER_VAR   = 2
+  };
+
   struct RowRef
   {
     Uint32 m_page_id;
@@ -159,7 +165,7 @@
     union
     {
       Uint16 unused;
-      Uint16 m_allocator;
+      enum Buffer_type m_alloc_type:16;
     };
 
     void copyto_link(Uint32 * dst) const {
@@ -198,7 +204,6 @@
   struct RowPtr
   {
     Uint32 m_type;
-    Uint32 m_src_node_no;
     Uint32 m_src_node_ptrI;
     Uint32 m_src_correlation;
 
@@ -234,8 +239,26 @@
     };
   };
 
-  struct SLFifoRowList
-  {
+  struct RowBuffer;  // forward decl.
+
+  /**
+   * Define overlayed 'base class' for SLFifoRowList and RowMap.
+   * As we want these to be POD struct, we does not use 
+   * inheritance, but have to take care that first part
+   * of these struct are correctly overlayed.
+   */
+  struct RowCollectionBase
+  {
+    RowBuffer* m_rowBuffer;
+  };
+
+  struct SLFifoRowList //: public RowCollectionBase
+  {
+    /**
+     * BEWARE: Overlayed 'struct RowCollectionBase'
+     */
+    RowBuffer* m_rowBuffer;
+
     /**
      * Data used for a single linked list of rows
      */
@@ -244,13 +267,22 @@
     Uint16 m_first_row_page_pos;
     Uint16 m_last_row_page_pos;
 
+    void construct(RowBuffer& rowBuffer) {
+      m_rowBuffer = &rowBuffer;
+      init();
+    }
     void init() { m_first_row_page_id = RNIL;}
     bool isNull() const { return m_first_row_page_id == RNIL; }
   };
 
-  struct RowMap
+  struct RowMap //: public RowCollectionBase
   {
     /**
+     * BEWARE: Overlayed 'struct RowCollectionBase'
+     */
+    RowBuffer* m_rowBuffer;
+
+    /**
      * Data used for a map with rows (key is correlation id)
      *   currently a single array is used to store row references
      *   (size == batch size)
@@ -259,7 +291,18 @@
     Uint16 m_size;                // size of array
     Uint16 m_elements;            // #elements in array
 
-    void init() { m_map_ref.setNull();}
+    void construct(RowBuffer& rowBuffer,
+                   Uint32 capacity)
+    {
+      m_rowBuffer = &rowBuffer;
+      m_size = capacity;
+      init();
+    }
+    void init() {
+      m_map_ref.setNull();
+      m_elements = 0;
+    }
+
     bool isNull() const { return m_map_ref.isNull(); }
 
     void assign (RowRef ref) {
@@ -296,36 +339,112 @@
     STATIC_CONST( MAP_SIZE_PER_REF_16 = 3 );
   };
 
-  struct SLFifoRowListIterator
-  {
-    RowRef m_ref;
-    Uint32 * m_row_ptr;
-
-    bool isNull() const { return m_ref.isNull(); }
-    void setNull() { m_ref.setNull(); }
-  };
-
-  struct SLFifoRowListIteratorPtr
-  {
-    RowRef m_ref;
-  };
-
-  struct RowMapIterator
-  {
-    Uint32 * m_row_ptr;
+  /**
+   * Define overlayed 'base class' for SLFifoRowListIterator
+   * and RowMapIterator.
+   * As we want these to be POD struct, we does not use 
+   * inheritance, but have to take care that first part
+   * of these struct are correctly overlayed.
+   */
+  struct RowIteratorBase
+  {
+    RowRef m_ref;
+    Uint32 * m_row_ptr;
+
+    bool isNull() const { return m_ref.isNull(); }
+    void setNull() { m_ref.setNull(); }
+  };
+
+  struct SLFifoRowListIterator //: public RowIteratorBase
+  {
+    /**
+     * BEWARE: Overlayed 'struct RowIteratorBase'
+     */
+    RowRef m_ref;
+    Uint32 * m_row_ptr;
+
+    bool isNull() const { return m_ref.isNull(); }
+    void setNull() { m_ref.setNull(); }
+    // END: RowIteratorBase
+  };
+
+  struct RowMapIterator //: public RowIteratorBase
+  {
+    /**
+     * BEWARE: Overlayed 'struct RowIteratorBase'
+     */
+    RowRef m_ref;
+    Uint32 * m_row_ptr;
+
+    bool isNull() const { return m_ref.isNull(); }
+    void setNull() { m_ref.setNull(); }
+    // END: RowIteratorBase
+
     Uint32 * m_map_ptr;
-    RowRef m_ref; // position of actual row
     Uint16 m_size;
     Uint16 m_element_no;
-    bool isNull() const { return m_ref.isNull(); }
-    void setNull() { m_ref.setNull(); }
-  };
-
-  struct RowMapIteratorPtr
-  {
-    Uint32 m_element_no;
-  };
-
+  };
+
+  /**
+   * Abstraction of SLFifoRowList & RowMap
+   */
+  struct RowCollection
+  {
+    enum collection_type
+    {
+      COLLECTION_VOID,
+      COLLECTION_MAP,
+      COLLECTION_LIST
+    };
+    union
+    {
+      RowCollectionBase m_base;  // Common part for map & list
+      SLFifoRowList m_list;
+      RowMap m_map;
+    };
+
+    RowCollection() : m_type(COLLECTION_VOID) {}
+
+    void construct(collection_type type,
+                   RowBuffer& rowBuffer,
+                   Uint32 capacity)
+    {
+      m_type = type;
+      if (m_type == COLLECTION_MAP)
+        m_map.construct(rowBuffer,capacity);
+      else if (m_type == COLLECTION_LIST)
+        m_list.construct(rowBuffer);
+    }
+
+    void init() {
+      if (m_type == COLLECTION_MAP)
+        m_map.init();
+      else if (m_type == COLLECTION_LIST)
+        m_list.init();
+    }
+
+    Uint32 rowOffset() const {
+      return (m_type == COLLECTION_MAP) ? 0 : 2;
+    }
+
+    collection_type m_type;
+  };
+
+  struct RowIterator
+  {
+    union
+    {
+      RowIteratorBase m_base;  // Common part for map & list
+      SLFifoRowListIterator m_list;
+      RowMapIterator m_map;
+    };
+    RowCollection::collection_type m_type;
+
+    RowIterator() { init(); }
+    void init() { m_base.setNull(); }
+    bool isNull() const { return m_base.isNull(); }
+  };
+ 
 
   /**
    * A struct used when building an TreeNode
@@ -368,11 +487,24 @@
 
   struct RowBuffer
   {
-    RowBuffer() { stack_init(); }
+    enum Buffer_type m_type;
+
+    RowBuffer() : m_type(BUFFER_VOID) {}
     DLFifoList<RowPage>::Head m_page_list;
 
-    void stack_init() { new (&m_page_list) DLFifoList<RowPage>::Head(); m_stack.m_pos = 0xFFFF; }
-    void var_init() { new (&m_page_list) DLFifoList<RowPage>::Head(); m_var.m_free = 0; }
+    void init(enum Buffer_type type)
+    {
+      new (&m_page_list) DLFifoList<RowPage>::Head();
+      m_type = type;
+      reset();
+    }
+    void reset()
+    {
+      if (m_type == BUFFER_STACK)
+        m_stack.m_pos = 0xFFFF;
+      else if (m_type == BUFFER_VAR)
+        m_var.m_free = 0;
+    }
 
     struct Stack
     {
@@ -840,11 +972,7 @@
     /**
      * Rows buffered by this node
      */
-    union
-    {
-      RowMap m_row_map;
-      SLFifoRowList m_row_list;
-    };
+    RowCollection m_rows;
 
     union
     {
@@ -892,7 +1020,7 @@
       RT_SCAN                = 0x1  // unbounded result set, scan interface
       ,RT_ROW_BUFFERS        = 0x2  // Do any of the node use row-buffering
       ,RT_MULTI_SCAN         = 0x4  // Is there several scans in request
-      ,RT_VAR_ALLOC          = 0x8  // Is var-allocation used for row-buffer
+//    ,RT_VAR_ALLOC          = 0x8  // DEPRECATED
       ,RT_NEED_PREPARE       = 0x10 // Does any node need m_prepare hook
       ,RT_NEED_COMPLETE      = 0x20 // Does any node need m_complete hook
       ,RT_REPEAT_SCAN_RESULT = 0x40 // Repeat bushy scan result when required
@@ -1122,6 +1250,7 @@
    */
   const OpInfo* getOpInfo(Uint32 op);
   Uint32 build(Build_context&,Ptr<Request>,SectionReader&,SectionReader&);
+  Uint32 initRowBuffers(Ptr<Request>);
   void checkPrepareComplete(Signal*, Ptr<Request>, Uint32 cnt);
   void start(Signal*, Ptr<Request>);
   void checkBatchComplete(Signal*, Ptr<Request>, Uint32 cnt);
@@ -1138,7 +1267,6 @@
   void releaseScanBuffers(Ptr<Request> requestPtr);
   void releaseRequestBuffers(Ptr<Request> requestPtr, bool reset);
   void releaseNodeRows(Ptr<Request> requestPtr, Ptr<TreeNode>);
-  void releaseRow(Ptr<Request>, RowRef ref);
   void registerActiveCursor(Ptr<Request>, Ptr<TreeNode>);
   void nodeFail_checkRequests(Signal*);
   void cleanup_common(Ptr<Request>, Ptr<TreeNode>);
@@ -1146,31 +1274,37 @@
   /**
    * Row buffering
    */
-  Uint32 storeRow(Ptr<Request>, Ptr<TreeNode>, RowPtr &row);
+  Uint32 storeRow(RowCollection& collection, RowPtr &row);
+  void releaseRow(RowCollection& collection, RowRef ref);
   Uint32* stackAlloc(RowBuffer& dst, RowRef&, Uint32 len);
   Uint32* varAlloc(RowBuffer& dst, RowRef&, Uint32 len);
-
-  void add_to_list(SLFifoRowList & list, RowRef rowref);
-  Uint32 add_to_map(Ptr<Request> requestPtr, Ptr<TreeNode>, Uint32, RowRef);
-  Uint32 * get_row_ptr(const RowMap&, RowMapIterator pos);
-  void setupRowPtr(Ptr<TreeNode>, RowPtr& dst, RowRef, const Uint32 * src);
-
-  // NOTE: ref contains info about it being stack/var
-  // so adding an inline would be nice...but that remove possibility
-  // to add jam()'s
-  Uint32 * get_row_ptr_stack(RowRef pos);
-  Uint32 * get_row_ptr_var(RowRef pos);
+  Uint32* rowAlloc(RowBuffer& dst, RowRef&, Uint32 len);
+
+  void add_to_list(SLFifoRowList & list, RowRef);
+  Uint32 add_to_map(RowMap& map, Uint32, RowRef);
+
+  void setupRowPtr(const RowCollection& collection,
+                   RowPtr& dst, RowRef, const Uint32 * src);
+  Uint32 * get_row_ptr(RowRef pos);
 
   /**
    * SLFifoRowListIterator
    */
-  bool first(Ptr<Request>, Ptr<TreeNode>, SLFifoRowListIterator&);
+  bool first(const SLFifoRowList& list, SLFifoRowListIterator&);
   bool next(SLFifoRowListIterator&);
-  bool next(Ptr<Request>, Ptr<TreeNode>, SLFifoRowListIterator&, SLFifoRowListIteratorPtr);
 
-  bool first(Ptr<Request>, Ptr<TreeNode>, RowMapIterator&);
+  /**
+   * RowMapIterator
+   */
+  bool first(const RowMap& map, RowMapIterator&);
   bool next(RowMapIterator&);
-  bool next(Ptr<Request>,Ptr<TreeNode>, RowMapIterator&, RowMapIteratorPtr);
+
+  /**
+   * RowIterator:
+   * Abstraction which may iterate either a RowList or Map
+   */
+  bool first(const RowCollection&, RowIterator&);
+  bool next(RowIterator&);
 
   /**
    * Misc

=== modified file 'storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp'
--- a/storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp	2012-10-04 11:27:10 +0000
+++ b/storage/ndb/src/kernel/blocks/dbspj/DbspjMain.cpp	2012-11-08 11:55:49 +0000
@@ -44,34 +44,31 @@
 #include <signaldata/ReadNodesConf.hpp>
 #include <signaldata/SignalDroppedRep.hpp>
 
-// Use DEBUG to print messages that should be
-// seen only when we debug the product
-
 #ifdef VM_TRACE
 
+/**
+ * DEBUG options for different parts od SPJ block
+ * Comment out those part you don't want DEBUG'ed.
+ */
 #define DEBUG(x) ndbout << "DBSPJ: "<< x << endl;
-#define DEBUG_DICT(x) ndbout << "DBSPJ: "<< x << endl;
-#define DEBUG_LQHKEYREQ
-#define DEBUG_SCAN_FRAGREQ
-
-#else
-
+//#define DEBUG_DICT(x) ndbout << "DBSPJ: "<< x << endl;
+//#define DEBUG_LQHKEYREQ
+//#define DEBUG_SCAN_FRAGREQ
+#endif
+
+/**
+ * Provide empty defs for those DEBUGs which has to be defined.
+ */
+#if !defined(DEBUG)
 #define DEBUG(x)
+#endif
+
+#if !defined(DEBUG_DICT)
 #define DEBUG_DICT(x)
-
 #endif
 
 #define DEBUG_CRASH() ndbassert(false)
 
-#if 1
-#undef DEBUG
-#define DEBUG(x)
-#undef DEBUG_DICT
-#define DEBUG_DICT(x)
-#undef DEBUG_LQHKEYREQ
-#undef DEBUG_SCAN_FRAGREQ
-#endif
-
 const Ptr<Dbspj::TreeNode> Dbspj::NullTreeNodePtr = { 0, RNIL };
 const Dbspj::RowRef Dbspj::NullRowRef = { RNIL, GLOBAL_PAGE_SIZE_WORDS, { 0 } };
 
@@ -1289,54 +1286,92 @@
   }
   requestPtr.p->m_node_cnt = ctx.m_cnt;
 
-  /**
-   * Init ROW_BUFFERS for those TreeNodes requiring either
-   * T_ROW_BUFFER or T_ROW_BUFFER_MAP.
-   */
-  if (requestPtr.p->m_bits & Request::RT_ROW_BUFFERS)
-  {
-    Ptr<TreeNode> treeNodePtr;
-    Local_TreeNode_list list(m_treenode_pool, requestPtr.p->m_nodes);
-    for (list.first(treeNodePtr); !treeNodePtr.isNull(); list.next(treeNodePtr))
-    {
-      if (treeNodePtr.p->m_bits & TreeNode::T_ROW_BUFFER_MAP)
-      {
-        jam();
-        treeNodePtr.p->m_row_map.init();
-      }
-      else if (treeNodePtr.p->m_bits & TreeNode::T_ROW_BUFFER)
-      {
-        jam();
-        treeNodePtr.p->m_row_list.init();
-      }
-    }
-  }
-
   if (ctx.m_scan_cnt > 1)
   {
     jam();
     requestPtr.p->m_bits |= Request::RT_MULTI_SCAN;
+  }
+
+  // Construct RowBuffers where required
+  err = initRowBuffers(requestPtr);
+  if (unlikely(err != 0))
+  {
+    jam();
+    goto error;
+  }
+
+  return 0;
+
+error:
+  jam();
+  return err;
+}
+
+/**
+ * initRowBuffers will decide row-buffering strategy, and init
+ * the RowBuffers where required.
+ */
+Uint32
+Dbspj::initRowBuffers(Ptr<Request> requestPtr)
+{
+  Local_TreeNode_list list(m_treenode_pool, requestPtr.p->m_nodes);
+
+  /**
+   * Init ROW_BUFFERS iff Request has to buffer any rows.
+   */
+  if (requestPtr.p->m_bits & Request::RT_ROW_BUFFERS)
+  {
+    jam();
 
     /**
      * Iff, multi-scan is non-bushy (normal case)
-     *   we don't strictly need RT_VAR_ALLOC for RT_ROW_BUFFERS
+     *   we don't strictly need BUFFER_VAR for RT_ROW_BUFFERS
      *   but could instead pop-row stack frame,
      *     however this is not implemented...
      *
-     * so, use RT_VAR_ALLOC
+     * so, currently use BUFFER_VAR if 'RT_MULTI_SCAN'
+     *
+     * NOTE: This should easily be solvable by having a 
+     *       RowBuffer for each TreeNode instead
      */
-    if (requestPtr.p->m_bits & Request::RT_ROW_BUFFERS)
-    {
-      jam();
-      requestPtr.p->m_bits |= Request::RT_VAR_ALLOC;
+    if (requestPtr.p->m_bits & Request::RT_MULTI_SCAN)
+    {
+      jam();
+      requestPtr.p->m_rowBuffer.init(BUFFER_VAR);
+    }
+    else
+    {
+      jam();
+      requestPtr.p->m_rowBuffer.init(BUFFER_STACK);
+    }
+
+    Ptr<TreeNode> treeNodePtr;
+    for (list.first(treeNodePtr); !treeNodePtr.isNull(); list.next(treeNodePtr))
+    {
+      jam();
+      ndbassert(treeNodePtr.p->m_batch_size > 0);
+      /**
+       * Construct a List or Map RowCollection for those TreeNodes
+       * requiring rows to be buffered.
+       */
+      if (treeNodePtr.p->m_bits & TreeNode::T_ROW_BUFFER_MAP)
+      {
+        jam();
+        treeNodePtr.p->m_rows.construct (RowCollection::COLLECTION_MAP,
+                                         requestPtr.p->m_rowBuffer,
+                                         treeNodePtr.p->m_batch_size);
+      }
+      else if (treeNodePtr.p->m_bits & TreeNode::T_ROW_BUFFER)
+      {
+        jam();
+        treeNodePtr.p->m_rows.construct (RowCollection::COLLECTION_LIST,
+                                         requestPtr.p->m_rowBuffer,
+                                         treeNodePtr.p->m_batch_size);
+      }
     }
   }
 
   return 0;
-
-error:
-  jam();
-  return err;
 }
 
 Uint32
@@ -1859,64 +1894,46 @@
      << ", request: " << requestPtr.i
   );
 
-  // only when var-alloc, or else stack will be popped wo/ consideration
-  // to individual rows
-  ndbassert(requestPtr.p->m_bits & Request::RT_VAR_ALLOC);
   ndbassert(treeNodePtr.p->m_bits & TreeNode::T_ROW_BUFFER);
 
-  /**
-   * Two ways to iterate...
-   */
-  if ((treeNodePtr.p->m_bits & TreeNode::T_ROW_BUFFER_MAP) == 0)
+  Uint32 cnt = 0;
+  RowIterator iter;
+  for (first(treeNodePtr.p->m_rows, iter); !iter.isNull(); )
   {
     jam();
-    Uint32 cnt = 0;
-    SLFifoRowListIterator iter;
-    for (first(requestPtr, treeNodePtr, iter); !iter.isNull(); )
-    {
-      jam();
-      RowRef pos = iter.m_ref;
-      next(iter);
-      releaseRow(requestPtr, pos);
-      cnt ++;
-    }
-    treeNodePtr.p->m_row_list.init();
-    DEBUG("SLFifoRowListIterator: released " << cnt << " rows!");
+    RowRef pos = iter.m_base.m_ref;
+    next(iter);
+    releaseRow(treeNodePtr.p->m_rows, pos);
+    cnt ++;
   }
-  else
+  treeNodePtr.p->m_rows.init();
+  DEBUG("RowIterator: released " << cnt << " rows!");
+
+  if (treeNodePtr.p->m_rows.m_type == RowCollection::COLLECTION_MAP)
   {
     jam();
-    Uint32 cnt = 0;
-    RowMapIterator iter;
-    for (first(requestPtr, treeNodePtr, iter); !iter.isNull(); )
-    {
-      jam();
-      RowRef pos = iter.m_ref;
-      // this could be made more efficient by not actually seting up m_row_ptr
-      next(iter);
-      releaseRow(requestPtr, pos);
-      cnt++;
-    }
-
     // Release the (now empty) RowMap
-    RowMap& map = treeNodePtr.p->m_row_map;
+    RowMap& map = treeNodePtr.p->m_rows.m_map;
     if (!map.isNull())
     {
       jam();
       RowRef ref;
       map.copyto(ref);
-      releaseRow(requestPtr, ref);  // Map was allocated in row memory
-      map.init();
+      releaseRow(treeNodePtr.p->m_rows, ref);  // Map was allocated in row memory
     }
-    DEBUG("RowMapIterator: released " << cnt << " rows!");
   }
 }
 
 void
-Dbspj::releaseRow(Ptr<Request> requestPtr, RowRef pos)
+Dbspj::releaseRow(RowCollection& collection, RowRef pos)
 {
-  ndbassert(requestPtr.p->m_bits & Request::RT_VAR_ALLOC);
-  ndbassert(pos.m_allocator == 1);
+  // only when var-alloc, or else stack will be popped wo/ consideration
+  // to individual rows
+  ndbassert(collection.m_base.m_rowBuffer != NULL);
+  ndbassert(collection.m_base.m_rowBuffer->m_type == BUFFER_VAR);
+  ndbassert(pos.m_alloc_type == BUFFER_VAR);
+
+  RowBuffer& rowBuffer = *collection.m_base.m_rowBuffer;
   Ptr<RowPage> ptr;
   m_page_pool.getPtr(ptr, pos.m_page_id);
   ((Var_page*)ptr.p)->free_record(pos.m_page_pos, Var_page::CHAIN);
@@ -1925,7 +1942,7 @@
   {
     jam();
     LocalDLFifoList<RowPage> list(m_page_pool,
-                                  requestPtr.p->m_rowBuffer.m_page_list);
+                                  rowBuffer.m_page_list);
     const bool last = list.hasNext(ptr) == false;
     list.remove(ptr);
     if (list.isEmpty())
@@ -1935,7 +1952,7 @@
        * Don't remove last page...
        */
       list.addLast(ptr);
-      requestPtr.p->m_rowBuffer.m_var.m_free = free_space;
+      rowBuffer.m_var.m_free = free_space;
     }
     else
     {
@@ -1948,20 +1965,19 @@
          */
         Ptr<RowPage> newLastPtr;
         ndbrequire(list.last(newLastPtr));
-        requestPtr.p->m_rowBuffer.m_var.m_free =
-          ((Var_page*)newLastPtr.p)->free_space;
+        rowBuffer.m_var.m_free = ((Var_page*)newLastPtr.p)->free_space;
       }
       releasePage(ptr);
     }
   }
-  else if (free_space > requestPtr.p->m_rowBuffer.m_var.m_free)
+  else if (free_space > rowBuffer.m_var.m_free)
   {
     jam();
     LocalDLFifoList<RowPage> list(m_page_pool,
-                                  requestPtr.p->m_rowBuffer.m_page_list);
+                                  rowBuffer.m_page_list);
     list.remove(ptr);
     list.addLast(ptr);
-    requestPtr.p->m_rowBuffer.m_var.m_free = free_space;
+    rowBuffer.m_var.m_free = free_space;
   }
 }
 
@@ -1988,7 +2004,7 @@
         list.remove();
       }
     }
-    requestPtr.p->m_rowBuffer.stack_init();
+    requestPtr.p->m_rowBuffer.reset();
   }
 
   if (reset)
@@ -1998,19 +2014,7 @@
     for (list.first(nodePtr); !nodePtr.isNull(); list.next(nodePtr))
     {
       jam();
-      if (nodePtr.p->m_bits & TreeNode::T_ROW_BUFFER)
-      {
-        jam();
-        if (nodePtr.p->m_bits & TreeNode::T_ROW_BUFFER_MAP)
-        {
-          jam();
-          nodePtr.p->m_row_map.init();
-        }
-        else
-        {
-          nodePtr.p->m_row_list.init();
-        }
-      }
+      nodePtr.p->m_rows.init();
     }
   }
 }
@@ -2569,6 +2573,11 @@
   {
     jam();
     Uint32 err;
+
+    DEBUG("Need to storeRow"
+      << ", node: " << treeNodePtr.p->m_node_no
+    );
+
     if (ERROR_INSERTED(17120) ||
        (ERROR_INSERTED(17121) && treeNodePtr.p->m_parentPtrI != RNIL))
     {
@@ -2576,7 +2585,7 @@
       CLEAR_ERROR_INSERT_VALUE;
       abort(signal, requestPtr, DbspjErr::OutOfRowMemory);
     }
-    else if ((err = storeRow(requestPtr, treeNodePtr, row)) != 0)
+    else if ((err = storeRow(treeNodePtr.p->m_rows, row)) != 0)
     {
       jam();
       abort(signal, requestPtr, err);
@@ -2593,60 +2602,43 @@
 }
 
 Uint32
-Dbspj::storeRow(Ptr<Request> requestPtr, Ptr<TreeNode> treeNodePtr, RowPtr &row)
+Dbspj::storeRow(RowCollection& collection, RowPtr &row)
 {
   ndbassert(row.m_type == RowPtr::RT_SECTION);
   SegmentedSectionPtr dataPtr = row.m_row_data.m_section.m_dataPtr;
   Uint32 * headptr = (Uint32*)row.m_row_data.m_section.m_header;
   Uint32 headlen = 1 + row.m_row_data.m_section.m_header->m_len;
 
-  DEBUG("storeRow"
-     << ", node: " << treeNodePtr.p->m_node_no
-     << ", request: " << requestPtr.i
-  );
-
   /**
-   * If rows are not in map, then they are kept in linked list
+   * Rows might be stored at an offset within the collection.
    */
-  Uint32 linklen = (treeNodePtr.p->m_bits & TreeNode::T_ROW_BUFFER_MAP)?
-    0 : 2;
+  const Uint32 offset = collection.rowOffset();
 
   Uint32 totlen = 0;
   totlen += dataPtr.sz;
   totlen += headlen;
-  totlen += linklen;
+  totlen += offset;
 
   RowRef ref;
-  Uint32 * dstptr = 0;
-  if ((requestPtr.p->m_bits & Request::RT_VAR_ALLOC) == 0)
-  {
-    jam();
-    dstptr = stackAlloc(requestPtr.p->m_rowBuffer, ref, totlen);
-  }
-  else
-  {
-    jam();
-    dstptr = varAlloc(requestPtr.p->m_rowBuffer, ref, totlen);
-  }
-
+  Uint32* const dstptr = rowAlloc(*collection.m_base.m_rowBuffer, ref, totlen);
   if (unlikely(dstptr == 0))
   {
     jam();
     return DbspjErr::OutOfRowMemory;
   }
-  memcpy(dstptr + linklen, headptr, 4 * headlen);
-  copy(dstptr + linklen + headlen, dataPtr);
+  memcpy(dstptr + offset, headptr, 4 * headlen);
+  copy(dstptr + offset + headlen, dataPtr);
 
-  if (linklen)
+  if (collection.m_type == RowCollection::COLLECTION_LIST)
   {
     jam();
     NullRowRef.copyto_link(dstptr); // Null terminate list...
-    add_to_list(treeNodePtr.p->m_row_list, ref);
+    add_to_list(collection.m_list, ref);
   }
   else
   {
     jam();
-    Uint32 error = add_to_map(requestPtr, treeNodePtr, row.m_src_correlation, ref);
+    Uint32 error = add_to_map(collection.m_map, row.m_src_correlation, ref);
     if (unlikely(error))
       return error;
   }
@@ -2656,30 +2648,17 @@
    * as above add_to_xxx may mave reorganized memory causing
    * alloced row to be moved.
    */
-  Uint32 * rowptr = 0;
-  if (ref.m_allocator == 0)
-  {
-    jam();
-    rowptr = get_row_ptr_stack(ref);
-  }
-  else
-  {
-    jam();
-    rowptr = get_row_ptr_var(ref);
-  }
-
-//ndbrequire(rowptr==dstptr);  // It moved which we now do handle
-  setupRowPtr(treeNodePtr, row, ref, rowptr);
+  const Uint32* const rowptr = get_row_ptr(ref);
+  setupRowPtr(collection, row, ref, rowptr);
   return 0;
 }
 
 void
-Dbspj::setupRowPtr(Ptr<TreeNode> treeNodePtr,
+Dbspj::setupRowPtr(const RowCollection& collection,
                    RowPtr& row, RowRef ref, const Uint32 * src)
 {
-  Uint32 linklen = (treeNodePtr.p->m_bits & TreeNode::T_ROW_BUFFER_MAP)?
-    0 : 2;
-  const RowPtr::Header * headptr = (RowPtr::Header*)(src + linklen);
+  const Uint32 offset = collection.rowOffset();
+  const RowPtr::Header * headptr = (RowPtr::Header*)(src + offset);
   Uint32 headlen = 1 + headptr->m_len;
 
   row.m_type = RowPtr::RT_LINEAR;
@@ -2704,20 +2683,10 @@
      * add last to list
      */
     RowRef last;
-    last.m_allocator = rowref.m_allocator;
+    last.m_alloc_type = rowref.m_alloc_type;
     last.m_page_id = list.m_last_row_page_id;
     last.m_page_pos = list.m_last_row_page_pos;
-    Uint32 * rowptr;
-    if (rowref.m_allocator == 0)
-    {
-      jam();
-      rowptr = get_row_ptr_stack(last);
-    }
-    else
-    {
-      jam();
-      rowptr = get_row_ptr_var(last);
-    }
+    Uint32 * const rowptr = get_row_ptr(last);
     rowref.copyto_link(rowptr);
   }
 
@@ -2726,29 +2695,28 @@
 }
 
 Uint32 *
-Dbspj::get_row_ptr_stack(RowRef pos)
-{
-  ndbassert(pos.m_allocator == 0);
-  Ptr<RowPage> ptr;
-  m_page_pool.getPtr(ptr, pos.m_page_id);
-  return ptr.p->m_data + pos.m_page_pos;
-}
-
-Uint32 *
-Dbspj::get_row_ptr_var(RowRef pos)
-{
-  ndbassert(pos.m_allocator == 1);
-  Ptr<RowPage> ptr;
-  m_page_pool.getPtr(ptr, pos.m_page_id);
-  return ((Var_page*)ptr.p)->get_ptr(pos.m_page_pos);
-}
-
+Dbspj::get_row_ptr(RowRef pos)
+{
+  Ptr<RowPage> ptr;
+  m_page_pool.getPtr(ptr, pos.m_page_id);
+  if (pos.m_alloc_type == BUFFER_STACK) // ::stackAlloc() memory
+  {
+    jam();
+    return ptr.p->m_data + pos.m_page_pos;
+  }
+  else                                 // ::varAlloc() memory
+  {
+    jam();
+    ndbassert(pos.m_alloc_type == BUFFER_VAR);
+    return ((Var_page*)ptr.p)->get_ptr(pos.m_page_pos);
+  }
+}
+
+inline
 bool
-Dbspj::first(Ptr<Request> requestPtr, Ptr<TreeNode> treeNodePtr,
+Dbspj::first(const SLFifoRowList& list,
              SLFifoRowListIterator& iter)
 {
-  Uint32 var = (requestPtr.p->m_bits & Request::RT_VAR_ALLOC) != 0;
-  SLFifoRowList & list = treeNodePtr.p->m_row_list;
   if (list.isNull())
   {
     jam();
@@ -2756,23 +2724,15 @@
     return false;
   }
 
-  iter.m_ref.m_allocator = var;
+  //  const Buffer_type allocator = list.m_rowBuffer->m_type;
+  iter.m_ref.m_alloc_type = list.m_rowBuffer->m_type;
   iter.m_ref.m_page_id = list.m_first_row_page_id;
   iter.m_ref.m_page_pos = list.m_first_row_page_pos;
-  if (var == 0)
-  {
-    jam();
-    iter.m_row_ptr = get_row_ptr_stack(iter.m_ref);
-  }
-  else
-  {
-    jam();
-    iter.m_row_ptr = get_row_ptr_var(iter.m_ref);
-  }
-
+  iter.m_row_ptr = get_row_ptr(iter.m_ref);
   return true;
 }
 
+inline
 bool
 Dbspj::next(SLFifoRowListIterator& iter)
 {
@@ -2782,63 +2742,25 @@
     jam();
     return false;
   }
-
-  if (iter.m_ref.m_allocator == 0)
-  {
-    jam();
-    iter.m_row_ptr = get_row_ptr_stack(iter.m_ref);
-  }
-  else
-  {
-    jam();
-    iter.m_row_ptr = get_row_ptr_var(iter.m_ref);
-  }
+  iter.m_row_ptr = get_row_ptr(iter.m_ref);
   return true;
 }
 
-bool
-Dbspj::next(Ptr<Request> requestPtr, Ptr<TreeNode> treeNodePtr,
-            SLFifoRowListIterator& iter, SLFifoRowListIteratorPtr start)
-{
-  Uint32 var = (requestPtr.p->m_bits & Request::RT_VAR_ALLOC) != 0;
-  (void)var;
-  ndbassert(var == iter.m_ref.m_allocator);
-  if (iter.m_ref.m_allocator == 0)
-  {
-    jam();
-    iter.m_row_ptr = get_row_ptr_stack(start.m_ref);
-  }
-  else
-  {
-    jam();
-    iter.m_row_ptr = get_row_ptr_var(start.m_ref);
-  }
-  return next(iter);
-}
-
 Uint32
-Dbspj::add_to_map(Ptr<Request> requestPtr, Ptr<TreeNode> treeNodePtr,
+Dbspj::add_to_map(RowMap& map,
                   Uint32 corrVal, RowRef rowref)
 {
   Uint32 * mapptr;
-  RowMap& map = treeNodePtr.p->m_row_map;
   if (map.isNull())
   {
     jam();
-    Uint16 batchsize = treeNodePtr.p->m_batch_size;
-    Uint32 sz16 = RowMap::MAP_SIZE_PER_REF_16 * batchsize;
+    ndbassert(map.m_size > 0);
+    ndbassert(map.m_rowBuffer != NULL);
+
+    Uint32 sz16 = RowMap::MAP_SIZE_PER_REF_16 * map.m_size;
     Uint32 sz32 = (sz16 + 1) / 2;
     RowRef ref;
-    if ((requestPtr.p->m_bits & Request::RT_VAR_ALLOC) == 0)
-    {
-      jam();
-      mapptr = stackAlloc(requestPtr.p->m_rowBuffer, ref, sz32);
-    }
-    else
-    {
-      jam();
-      mapptr = varAlloc(requestPtr.p->m_rowBuffer, ref, sz32);
-    }
+    mapptr = rowAlloc(*map.m_rowBuffer, ref, sz32);
     if (unlikely(mapptr == 0))
     {
       jam();
@@ -2846,7 +2768,6 @@
     }
     map.assign(ref);
     map.m_elements = 0;
-    map.m_size = batchsize;
     map.clear(mapptr);
   }
   else
@@ -2854,16 +2775,7 @@
     jam();
     RowRef ref;
     map.copyto(ref);
-    if (ref.m_allocator == 0)
-    {
-      jam();
-      mapptr = get_row_ptr_stack(ref);
-    }
-    else
-    {
-      jam();
-      mapptr = get_row_ptr_var(ref);
-    }
+    mapptr = get_row_ptr(ref);
   }
 
   Uint32 pos = corrVal & 0xFFFF;
@@ -2885,12 +2797,11 @@
   return 0;
 }
 
+inline
 bool
-Dbspj::first(Ptr<Request> requestPtr, Ptr<TreeNode> treeNodePtr,
+Dbspj::first(const RowMap& map,
              RowMapIterator & iter)
 {
-  Uint32 var = (requestPtr.p->m_bits & Request::RT_VAR_ALLOC) != 0;
-  RowMap& map = treeNodePtr.p->m_row_map;
   if (map.isNull())
   {
     jam();
@@ -2898,18 +2809,9 @@
     return false;
   }
 
-  if (var == 0)
-  {
-    jam();
-    iter.m_map_ptr = get_row_ptr_stack(map.m_map_ref);
-  }
-  else
-  {
-    jam();
-    iter.m_map_ptr = get_row_ptr_var(map.m_map_ref);
-  }
+  iter.m_map_ptr = get_row_ptr(map.m_map_ref);
   iter.m_size = map.m_size;
-  iter.m_ref.m_allocator = var;
+  iter.m_ref.m_alloc_type = map.m_rowBuffer->m_type;
 
   Uint32 pos = 0;
   while (RowMap::isNull(iter.m_map_ptr, pos) && pos < iter.m_size)
@@ -2926,20 +2828,12 @@
     jam();
     RowMap::load(iter.m_map_ptr, pos, iter.m_ref);
     iter.m_element_no = pos;
-    if (var == 0)
-    {
-      jam();
-      iter.m_row_ptr = get_row_ptr_stack(iter.m_ref);
-    }
-    else
-    {
-      jam();
-      iter.m_row_ptr = get_row_ptr_var(iter.m_ref);
-    }
+    iter.m_row_ptr = get_row_ptr(iter.m_ref);
     return true;
   }
 }
 
+inline
 bool
 Dbspj::next(RowMapIterator & iter)
 {
@@ -2958,45 +2852,46 @@
     jam();
     RowMap::load(iter.m_map_ptr, pos, iter.m_ref);
     iter.m_element_no = pos;
-    if (iter.m_ref.m_allocator == 0)
-    {
-      jam();
-      iter.m_row_ptr = get_row_ptr_stack(iter.m_ref);
-    }
-    else
-    {
-      jam();
-      iter.m_row_ptr = get_row_ptr_var(iter.m_ref);
-    }
+    iter.m_row_ptr = get_row_ptr(iter.m_ref);
     return true;
   }
 }
 
 bool
-Dbspj::next(Ptr<Request> requestPtr, Ptr<TreeNode> treeNodePtr,
-            RowMapIterator & iter, RowMapIteratorPtr start)
-{
-  Uint32 var = (requestPtr.p->m_bits & Request::RT_VAR_ALLOC) != 0;
-  RowMap& map = treeNodePtr.p->m_row_map;
-  ndbrequire(!map.isNull());
-
-  if (var == 0)
-  {
-    jam();
-    iter.m_map_ptr = get_row_ptr_stack(map.m_map_ref);
-  }
-  else
-  {
-    jam();
-    iter.m_map_ptr = get_row_ptr_var(map.m_map_ref);
-  }
-  iter.m_size = map.m_size;
-
-  RowMap::load(iter.m_map_ptr, start.m_element_no, iter.m_ref);
-  iter.m_element_no = start.m_element_no;
-  return next(iter);
-}
-
+Dbspj::first(const RowCollection& collection,
+             RowIterator& iter)
+{
+  iter.m_type = collection.m_type;
+  if (iter.m_type == RowCollection::COLLECTION_LIST)
+  {
+    jam();
+    return first(collection.m_list, iter.m_list);
+  }
+  else
+  {
+    jam();
+    ndbassert(iter.m_type == RowCollection::COLLECTION_MAP);
+    return first(collection.m_map, iter.m_map);
+  }
+}
+
+bool
+Dbspj::next(RowIterator& iter)
+{
+  if (iter.m_type == RowCollection::COLLECTION_LIST)
+  {
+    jam();
+    return next(iter.m_list);
+  }
+  else
+  {
+    jam();
+    ndbassert(iter.m_type == RowCollection::COLLECTION_MAP);
+    return next(iter.m_map);
+  }
+}
+
+inline
 Uint32 *
 Dbspj::stackAlloc(RowBuffer & buffer, RowRef& dst, Uint32 sz)
 {
@@ -3025,11 +2920,12 @@
 
   dst.m_page_id = ptr.i;
   dst.m_page_pos = pos;
-  dst.m_allocator = 0;
+  dst.m_alloc_type = BUFFER_STACK;
   buffer.m_stack.m_pos = pos + sz;
   return ptr.p->m_data + pos;
 }
 
+inline
 Uint32 *
 Dbspj::varAlloc(RowBuffer & buffer, RowRef& dst, Uint32 sz)
 {
@@ -3061,11 +2957,32 @@
 
   dst.m_page_id = ptr.i;
   dst.m_page_pos = pos;
-  dst.m_allocator = 1;
+  dst.m_alloc_type = BUFFER_VAR;
   buffer.m_var.m_free = vp->free_space;
   return vp->get_ptr(pos);
 }
 
+Uint32 *
+Dbspj::rowAlloc(RowBuffer& rowBuffer, RowRef& dst, Uint32 sz)
+{
+  if (rowBuffer.m_type == BUFFER_STACK)
+  {
+    jam();
+    return stackAlloc(rowBuffer, dst, sz);
+  }
+  else if (rowBuffer.m_type == BUFFER_VAR)
+  {
+    jam();
+    return varAlloc(rowBuffer, dst, sz);
+  }
+  else
+  {
+    jam();
+    ndbrequire(false);
+    return NULL;
+  }
+}
+
 bool
 Dbspj::allocPage(Ptr<RowPage> & ptr)
 {
@@ -7523,49 +7440,28 @@
     m_treenode_pool.getPtr(treeNodePtr, treeNodePtr.p->m_parentPtrI);
     DEBUG("appendFromParent"
        << ", node: " << treeNodePtr.p->m_node_no);
-    if (unlikely((treeNodePtr.p->m_bits & TreeNode::T_ROW_BUFFER_MAP) == 0))
+    if (unlikely(treeNodePtr.p->m_rows.m_type != RowCollection::COLLECTION_MAP))
     {
       DEBUG_CRASH();
       return DbspjErr::InvalidPattern;
     }
 
     RowRef ref;
-    treeNodePtr.p->m_row_map.copyto(ref);
-    Uint32 allocator = ref.m_allocator;
-    const Uint32 * mapptr;
-    if (allocator == 0)
-    {
-      jam();
-      mapptr = get_row_ptr_stack(ref);
-    }
-    else
-    {
-      jam();
-      mapptr = get_row_ptr_var(ref);
-    }
+    treeNodePtr.p->m_rows.m_map.copyto(ref);
+    const Uint32* const mapptr = get_row_ptr(ref);
 
     Uint32 pos = corrVal >> 16; // parent corr-val
-    if (unlikely(! (pos < treeNodePtr.p->m_row_map.m_size)))
+    if (unlikely(! (pos < treeNodePtr.p->m_rows.m_map.m_size)))
     {
       DEBUG_CRASH();
       return DbspjErr::InvalidPattern;
     }
 
     // load ref to parent row
-    treeNodePtr.p->m_row_map.load(mapptr, pos, ref);
+    treeNodePtr.p->m_rows.m_map.load(mapptr, pos, ref);
 
-    const Uint32 * rowptr;
-    if (allocator == 0)
-    {
-      jam();
-      rowptr = get_row_ptr_stack(ref);
-    }
-    else
-    {
-      jam();
-      rowptr = get_row_ptr_var(ref);
-    }
-    setupRowPtr(treeNodePtr, targetRow, ref, rowptr);
+    const Uint32* const rowptr = get_row_ptr(ref);
+    setupRowPtr(treeNodePtr.p->m_rows, targetRow, ref, rowptr);
 
     if (levels)
     {

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.1-telco-7.0 branch (ole.john.aske:5028 to 5029)Bug#14709490Ole John Aske9 Nov