Boost logo

Boost-Commit :

Subject: [Boost-commit] svn:boost r56720 - in sandbox/stm/boost/stm: . detail
From: justin_at_[hidden]
Date: 2009-10-11 15:35:39


Author: jgottschlich
Date: 2009-10-11 15:35:38 EDT (Sun, 11 Oct 2009)
New Revision: 56720
URL: http://svn.boost.org/trac/boost/changeset/56720

Log:
Fixed race condition found by Vicente on member initialization
access to shared maps which could be modified by threads upon
entry and exit. The entry condition should not have caused errors
because of the assumption that no transactions were previously
started prior to all threads initializing. However, the thread
termination could have caused crashes. Now a light_auto_lock
object is initialized in the member initialization which prevents
both thread entry/exit and transaction construction from occurring
at the same time. This blocks the race condition of simultaneous,
shared access to the maps.

In addition, I have added the new code to the contention manager
which now makes a single virtual call listing a committing
transaction's conflicts with all other transactions. Previously,
it would make a virtual call per transactional conflict. If the
conflicts were high, the vcall overhead could cause performance
degradation since a committing transaction is a critical serialization
point.

Text files modified:
   sandbox/stm/boost/stm/base_transaction.hpp | 47 ++++++++++++++++++++++
   sandbox/stm/boost/stm/contention_manager.hpp | 84 ++++++++++++++++++++++++++++++++++++++++
   sandbox/stm/boost/stm/detail/transaction_impl.hpp | 48 +++++++++++++---------
   sandbox/stm/boost/stm/transaction.hpp | 9 ---
   4 files changed, 161 insertions(+), 27 deletions(-)

Modified: sandbox/stm/boost/stm/base_transaction.hpp
==============================================================================
--- sandbox/stm/boost/stm/base_transaction.hpp (original)
+++ sandbox/stm/boost/stm/base_transaction.hpp 2009-10-11 15:35:38 EDT (Sun, 11 Oct 2009)
@@ -168,6 +168,53 @@
    inline void unlock(PLOCK &lock) { lock.unlock(); }
    inline void unlock(PLOCK *lock) { lock->unlock(); }
 #endif
+
+
+//-----------------------------------------------------------------------------
+//-----------------------------------------------------------------------------
+class light_auto_lock
+{
+public:
+
+ light_auto_lock(Mutex &mutex) : lock_(NULL)
+ {
+ do_auto_lock(&mutex);
+ }
+
+ light_auto_lock(Mutex *mutex) : lock_(NULL)
+ {
+ do_auto_lock(mutex);
+ }
+
+ ~light_auto_lock() { do_auto_unlock(); }
+
+ bool has_lock() { return hasLock_; }
+
+ void release_lock() { do_auto_unlock(); }
+
+private:
+
+ void do_auto_lock(Mutex *mutex)
+ {
+ lock_ = mutex;
+ pthread_mutex_lock(mutex);
+ hasLock_ = true;
+ }
+
+ void do_auto_unlock()
+ {
+ if (hasLock_)
+ {
+ hasLock_ = false;
+ pthread_mutex_unlock(lock_);
+ }
+ }
+
+ bool hasLock_;
+ Mutex *lock_;
+};
+
+
 //-----------------------------------------------------------------------------
 //-----------------------------------------------------------------------------
 class base_transaction_object

Modified: sandbox/stm/boost/stm/contention_manager.hpp
==============================================================================
--- sandbox/stm/boost/stm/contention_manager.hpp (original)
+++ sandbox/stm/boost/stm/contention_manager.hpp 2009-10-11 15:35:38 EDT (Sun, 11 Oct 2009)
@@ -129,6 +129,90 @@
       //return lhs.writes() + lhs.reads() >= rhs.writes() + rhs.reads();
    }
 
+ virtual bool permission_to_abort
+ (boost::stm::transaction const &lhs,
+ std::list<boost::stm::transaction*> &rhs)
+ {
+#ifdef JUST_PRIORITY
+ int setSize = (lhs.writes() * lhs.priority()) +
+ (lhs.reads() * lhs.priority());
+ double abortSetSize = 0;
+ double abortPriority = 0;
+ double decrementing = 1.0;
+
+ double highestPriority = 0;
+
+ bool hasLargestReadSet = true;
+
+ for (std::list<core::transaction*>::iterator iter = rhs.begin();
+ iter != rhs.end(); ++iter)
+ {
+ if ((*iter)->priority() > highestPriority)
+ {
+ highestPriority = (*iter)->priority();
+ }
+
+ if ((*iter)->reads() > lhs.reads()) hasLargestReadSet = false;
+ if ((*iter)->writes() > 0) return true;
+
+ abortSetSize += (double)(*iter)->reads() / decrementing;
+ abortPriority += (double)(*iter)->priority() / decrementing;
+ decrementing += 0.5;
+ }
+
+ if (lhs.priority() >= highestPriority) return true;
+
+ if (hasLargestReadSet) return true;
+
+ if (setSize >= abortPriority + abortSetSize)
+ {
+ return true;
+ }
+ else
+ {
+ return false;
+ }
+
+#else
+ double setSize = (lhs.writes() * lhs.priority()) +
+ (lhs.reads() * lhs.priority());
+ double abortSetSize = 0;
+ double abortPriority = 0;
+ bool hasLargestReadSet = true;
+
+ int mem = lhs.reads() + (lhs.writes() * 10);
+
+ double decrementing = 1.0;
+ for (std::list<boost::stm::transaction*>::iterator iter = rhs.begin();
+ iter != rhs.end(); ++iter)
+ {
+ if ((*iter)->reads() > mem) hasLargestReadSet = false;
+
+ if ((*iter)->writes() > 0) return true;
+
+ if (lhs.reads() < (*iter)->reads() / 8 &&
+ lhs.priority() * 100 < (*iter)->priority()) return false;
+
+ abortSetSize += (double)(*iter)->reads() / decrementing;
+ abortPriority += (double)(*iter)->priority() / decrementing;
+ decrementing += 0.5;
+ }
+
+ if (hasLargestReadSet) return true;
+
+ if (setSize >=
+ (abortPriority / setSize) + (abortSetSize / setSize))
+ {
+ return true;
+ }
+ else
+ {
+ return false;
+ }
+#endif
+ }
+
+
    virtual bool allow_lock_to_abort_tx
    (int const & lockWaitTime, int const &lockAborted,
    bool txTryingToAbortIsIrrevocable, boost::stm::transaction const &rhs)

Modified: sandbox/stm/boost/stm/detail/transaction_impl.hpp
==============================================================================
--- sandbox/stm/boost/stm/detail/transaction_impl.hpp (original)
+++ sandbox/stm/boost/stm/detail/transaction_impl.hpp 2009-10-11 15:35:38 EDT (Sun, 11 Oct 2009)
@@ -421,7 +421,16 @@
 //
 //--------------------------------------------------------------------------
 inline boost::stm::transaction::transaction() :
+
+ //-----------------------------------------------------------------------
+ // These two lines are vitally important ... make sure they are always
+ // the first two members of the class so the member initialization order
+ // always initializes them first.
+ //-----------------------------------------------------------------------
+
    threadId_(THREAD_ID),
+ auto_general_lock_(general_lock()),
+
 #if USE_SINGLE_THREAD_CONTEXT_MAP
 ////////////////////////////////////////
    context_(*tss_context_map_.find(threadId_)->second),
@@ -491,12 +500,13 @@
 ////////////////////////////////////////
 #endif
 
-
    hasMutex_(0), priority_(0),
    state_(e_no_state),
    reads_(0),
    startTime_(time(NULL))
 {
+ auto_general_lock_.release_lock();
+
    if (direct_updating()) doIntervalDeletions();
 #if PERFORMING_LATM
    while (blocked()) { SLEEP(10) ; }
@@ -2115,13 +2125,7 @@
                   ("aborting committing transaction due to contention manager priority inversion");
                }
 #else
- if (cm_->permission_to_abort(*this, *t)) aborted.push_front(t);
- else
- {
- force_to_abort();
- throw aborted_transaction_exception
- ("aborting committing transaction due to contention manager priority inversion");
- }
+ aborted.push_front(t);
 #endif
             }
          }
@@ -2151,13 +2155,7 @@
             ("aborting committing transaction due to contention manager priority inversion");
          }
 #else
- if (cm_->permission_to_abort(*this, *t)) aborted.push_front(t);
- else
- {
- force_to_abort();
- throw aborted_transaction_exception
- ("aborting committing transaction due to contention manager priority inversion");
- }
+ aborted.push_front(t);
 #endif
       }
 #endif
@@ -2165,13 +2163,23 @@
 
    if (!aborted.empty())
    {
- // ok, forced to aborts are allowed, do them
- for (std::list<transaction*>::iterator k = aborted.begin(); k != aborted.end(); ++k)
+ if (cm_->permission_to_abort(*this, aborted))
       {
- (*k)->force_to_abort();
- }
+ // ok, forced to aborts are allowed, do them
+ for (std::list<transaction*>::iterator k = aborted.begin();
+ k != aborted.end(); ++k)
+ {
+ (*k)->force_to_abort();
+ }
 
- aborted.clear();
+ aborted.clear();
+ }
+ else
+ {
+ force_to_abort();
+ throw aborted_transaction_exception
+ ("aborting committing transaction by contention manager");
+ }
    }
 
    return true;

Modified: sandbox/stm/boost/stm/transaction.hpp
==============================================================================
--- sandbox/stm/boost/stm/transaction.hpp (original)
+++ sandbox/stm/boost/stm/transaction.hpp 2009-10-11 15:35:38 EDT (Sun, 11 Oct 2009)
@@ -118,13 +118,6 @@
 
    typedef std::multimap<size_t, MemoryContainerList > DeletionBuffer;
 
-#if 0
-#ifndef BOOST_STM_USE_BOOST_MUTEX
- typedef pthread_mutex_t Mutex;
-#else
- typedef boost::mutex Mutex;
-#endif
-#endif
     typedef std::set<Mutex*> MutexSet;
 
    typedef std::set<size_t> ThreadIdSet;
@@ -1335,6 +1328,8 @@
    //--------------------------------------------------------------------------
    size_t threadId_;
 
+ light_auto_lock auto_general_lock_;
+
    //--------------------------------------------------------------------------
    // ******** WARNING ******** MOVING threadId_ WILL BREAK TRANSACTION.
    // threadId_ MUST ALWAYS THE FIRST MEMBER OF THIS CLASS. THE MEMBER


Boost-Commit list run by bdawes at acm.org, david.abrahams at rcn.com, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk