List:Commits« Previous MessageNext Message »
From:Jan Wedvik Date:May 4 2011 9:36am
Subject:bzr push into mysql-5.1-telco-7.0 branch (jan.wedvik:4353 to 4354)
View as plain text  
 4354 Jan Wedvik	2011-05-04
      This commit makes the following changes to NdbObjectIdMap:
      
      1. It puts unmapped objects at the end of the free list instead of at the front.
      This postpones reuse of object ids as long as possible, and thus minimizes 
      the chance of sending a late arriving message to the wrong recipient.
      
      2. It removes a redundant mutex (the class was not thread safe anyway).
      
      3. It refactors the code a bit, hopefully making it more readable.

    modified:
      storage/ndb/src/ndbapi/Ndbinit.cpp
      storage/ndb/src/ndbapi/ObjectMap.cpp
      storage/ndb/src/ndbapi/ObjectMap.hpp
 4353 jonas oreland	2011-05-03
      ndb - try to make rpl_ndb_ui2 stable

    modified:
      mysql-test/suite/rpl_ndb/r/rpl_ndb_ui2.result
      mysql-test/suite/rpl_ndb/t/rpl_ndb_ui2.test
=== modified file 'storage/ndb/src/ndbapi/Ndbinit.cpp'
--- a/storage/ndb/src/ndbapi/Ndbinit.cpp	2011-04-15 06:29:59 +0000
+++ b/storage/ndb/src/ndbapi/Ndbinit.cpp	2011-05-04 09:35:08 +0000
@@ -201,8 +201,7 @@ NdbImpl::NdbImpl(Ndb_cluster_connection 
     m_transporter_facade(ndb_cluster_connection->m_impl.m_transporter_facade),
     m_dictionary(ndb),
     theCurrentConnectIndex(0),
-    theNdbObjectIdMap(m_transporter_facade->theMutexPtr,
-		      1024,1024),
+    theNdbObjectIdMap(1024,1024),
     theNoOfDBnodes(0),
     theWaiter(this),
     m_ev_op(0),

=== modified file 'storage/ndb/src/ndbapi/ObjectMap.cpp'
--- a/storage/ndb/src/ndbapi/ObjectMap.cpp	2011-04-07 14:02:50 +0000
+++ b/storage/ndb/src/ndbapi/ObjectMap.cpp	2011-05-04 09:35:08 +0000
@@ -18,13 +18,13 @@
 
 #include "ObjectMap.hpp"
 
-NdbObjectIdMap::NdbObjectIdMap(NdbMutex* mutex, Uint32 sz, Uint32 eSz)
+NdbObjectIdMap::NdbObjectIdMap(Uint32 sz, Uint32 eSz):
+  m_expandSize(eSz),
+  m_size(0),
+  m_firstFree(InvalidId),
+  m_lastFree(InvalidId),
+  m_map(0)
 {
-  m_size = 0;
-  m_firstFree = InvalidId;
-  m_map = 0;
-  m_mutex = mutex;
-  m_expandSize = eSz;
   expand(sz);
 #ifdef DEBUG_OBJECTMAP
   ndbout_c("NdbObjectIdMap:::NdbObjectIdMap(%u)", sz);
@@ -33,12 +33,14 @@ NdbObjectIdMap::NdbObjectIdMap(NdbMutex*
 
 NdbObjectIdMap::~NdbObjectIdMap()
 {
+  assert(checkConsistency());
   free(m_map);
+  m_map = NULL;
 }
 
 int NdbObjectIdMap::expand(Uint32 incSize)
 {
-  NdbMutex_Lock(m_mutex);
+  assert(checkConsistency());
   Uint32 newSize = m_size + incSize;
   MapEntry * tmp = (MapEntry*)realloc(m_map, newSize * sizeof(MapEntry));
 
@@ -46,20 +48,45 @@ int NdbObjectIdMap::expand(Uint32 incSiz
   {
     m_map = tmp;
     
-    for(Uint32 i = m_size; i < newSize; i++){
-      m_map[i].m_next = 2 * (i + 1) + 1;
+    for(Uint32 i = m_size; i < newSize-1; i++)
+    {
+      m_map[i].setNext(i+1);
     }
-    m_firstFree = (2 * m_size) + 1;
-    m_map[newSize-1].m_next = Uint32(InvalidId);
+    m_firstFree = m_size;
+    m_lastFree = newSize - 1;
+    m_map[newSize-1].setNext(InvalidId);
     m_size = newSize;
+    assert(checkConsistency());
   }
   else
   {
-    NdbMutex_Unlock(m_mutex);
     g_eventLogger->error("NdbObjectIdMap::expand: realloc(%u*%lu) failed",
                          newSize, sizeof(MapEntry));
     return -1;
   }
-  NdbMutex_Unlock(m_mutex);
   return 0;
 }
+
+bool NdbObjectIdMap::checkConsistency()
+{
+  if (m_firstFree == InvalidId)
+  {
+    for (Uint32 i = 0; i<m_size; i++)
+    {
+      if (m_map[i].isFree())
+      {
+        assert(false);
+        return false;
+      }
+    }
+    return true;
+  }
+
+  Uint32 i = m_firstFree;
+  while (m_map[i].getNext() != InvalidId)
+  {
+    i = m_map[i].getNext();
+  }
+  assert(i == m_lastFree);
+  return i == m_lastFree;
+}

=== modified file 'storage/ndb/src/ndbapi/ObjectMap.hpp'
--- a/storage/ndb/src/ndbapi/ObjectMap.hpp	2011-04-07 14:02:50 +0000
+++ b/storage/ndb/src/ndbapi/ObjectMap.hpp	2011-05-04 09:35:08 +0000
@@ -20,7 +20,6 @@
 #define NDB_OBJECT_ID_MAP_HPP
 
 #include <ndb_global.h>
-//#include <NdbMutex.h>
 #include <NdbOut.hpp>
 
 #include <EventLogger.hpp>
@@ -31,11 +30,11 @@ extern EventLogger * g_eventLogger;
 /**
   * Global ObjectMap
   */
-class NdbObjectIdMap //: NdbLockable
+class NdbObjectIdMap
 {
 public:
-  STATIC_CONST( InvalidId = ~(Uint32)0 );
-  NdbObjectIdMap(NdbMutex*, Uint32 initalSize = 128, Uint32 expandSize = 10);
+  STATIC_CONST( InvalidId = 0x7fffffff );
+  NdbObjectIdMap(Uint32 initalSize, Uint32 expandSize);
   ~NdbObjectIdMap();
 
   Uint32 map(void * object);
@@ -43,34 +42,75 @@ public:
   
   void * getObject(Uint32 id);
 private:
+  const Uint32 m_expandSize;
   Uint32 m_size;
-  Uint32 m_expandSize;
   Uint32 m_firstFree;
-  union MapEntry {
-     UintPtr m_next;
-     void * m_obj;
-  } * m_map;
+  /**
+   * We put released entries at the end of the free list. That way, we delay
+   * re-use of an object id as long as possible. This minimizes the chance
+   * of sending an incoming message to the wrong object because the recipient
+   * object id was reused. 
+   */
+  Uint32 m_lastFree;
+
+  class MapEntry
+  {
+  public:
+    bool isFree() const
+    { 
+      return (m_val & 1) == 1; 
+    }
+
+    Uint32 getNext() const
+    {
+      assert(isFree());
+      return static_cast<Uint32>(m_val >> 1);
+    }
+
+    void setNext(Uint32 next)
+    { 
+      m_val = (next << 1) | 1; 
+    }
+
+    void* getObj() const
+    {
+      assert((m_val & 3) == 0);
+      return reinterpret_cast<void*>(m_val);
+    }
+    
+    void setObj(void* obj)
+    { 
+      m_val = reinterpret_cast<UintPtr>(obj); 
+      assert((m_val & 3) == 0);
+    }
+    
+  private:
+    /**
+     * This holds either a pointer to a mapped object *or* the index of the
+     * next entry in the free list. If it is a pointer, then the two least
+     * significant bits should be zero (requiring all mapped objects to be
+     * four-byte aligned). If it is an index, then bit 0 should be set.
+     */ 
+    UintPtr m_val;
+  };
+
+  MapEntry* m_map;
 
-  NdbMutex * m_mutex;
   int expand(Uint32 newSize);
+  // For debugging purposes.
+  bool checkConsistency();
 };
 
 inline
 Uint32
-NdbObjectIdMap::map(void * object){
-  
-  //  lock();
-  assert((UintPtr(object) & 3) == 0);
-  
-  if(m_firstFree == Uint32(InvalidId) && expand(m_expandSize))
+NdbObjectIdMap::map(void * object)
+{
+  if(m_firstFree == InvalidId && expand(m_expandSize))
     return InvalidId;
   
-  Uint32 ff = m_firstFree >> 1;
-  assert(UintPtr(m_map[ff].m_next) == Uint32(m_map[ff].m_next));
-  m_firstFree = Uint32(m_map[ff].m_next);
-  m_map[ff].m_obj = object;
-  
-  //  unlock();
+  const Uint32 ff = m_firstFree;
+  m_firstFree = m_map[ff].getNext();
+  m_map[ff].setObj(object);
   
   DBUG_PRINT("info",("NdbObjectIdMap::map(0x%lx) %u", (long) object, ff<<2));
 
@@ -79,26 +119,37 @@ NdbObjectIdMap::map(void * object){
 
 inline
 void *
-NdbObjectIdMap::unmap(Uint32 id, void *object){
-
-  Uint32 i = id>>2;
+NdbObjectIdMap::unmap(Uint32 id, void *object)
+{
+  const Uint32 i = id>>2;
 
-  //  lock();
-  if(i < m_size){
-    void * obj = m_map[i].m_obj;
-    if (object == obj) {
-      m_map[i].m_next = m_firstFree;
-      m_firstFree = (2 * i) + 1;
-    } else {
+  assert(i < m_size);
+  if(i < m_size)
+  {
+    void * const obj = m_map[i].getObj();
+    if (object == obj) 
+    {
+      m_map[i].setNext(InvalidId);
+      if (m_firstFree == InvalidId)
+      {
+        m_firstFree = i;
+      }
+      else
+      {
+        m_map[m_lastFree].setNext(i);
+      }
+      m_lastFree = i;
+    } 
+    else 
+    {
       g_eventLogger->error("NdbObjectIdMap::unmap(%u, 0x%lx) obj=0x%lx",
                            id, (long) object, (long) obj);
       DBUG_PRINT("error",("NdbObjectIdMap::unmap(%u, 0x%lx) obj=0x%lx",
                           id, (long) object, (long) obj));
+      assert(false);
       return 0;
     }
     
-    //  unlock();
-    
     DBUG_PRINT("info",("NdbObjectIdMap::unmap(%u) obj=0x%lx", id, (long) obj));
     
     return obj;
@@ -107,12 +158,21 @@ NdbObjectIdMap::unmap(Uint32 id, void *o
 }
 
 inline void *
-NdbObjectIdMap::getObject(Uint32 id){
+NdbObjectIdMap::getObject(Uint32 id)
+{
   // DBUG_PRINT("info",("NdbObjectIdMap::getObject(%u) obj=0x%x", id,  m_map[id>>2].m_obj));
   id >>= 2;
-  if(id < m_size){
-    if ((m_map[id].m_next & 3) == 0)
-      return m_map[id].m_obj;
+  assert(id < m_size);
+  if(id < m_size)
+  {
+    if(m_map[id].isFree())
+    {
+      return 0;
+    }
+    else
+    {
+      return m_map[id].getObj();
+    }
   }
   return 0;
 }

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.1-telco-7.0 branch (jan.wedvik:4353 to 4354) Jan Wedvik4 May