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 Wedvik | 4 May |
