From ff71d74967e55ec15e15f17f979a426d3d35b69b Mon Sep 17 00:00:00 2001 From: Keir Mierle Date: Fri, 10 Aug 2012 17:05:15 -0700 Subject: [PATCH] Further encapsulate Ceres's Mutex class. There are cases where one wishes to link Ceres against another application or library which uses a mutex implementation with similar ancestry to the one in Ceres. In those cases there are problems due to macro interactions which can't be contained with namespaces. This further isolates the Ceres Mutex class by adding CERES_ prefix to all the macros and also working around a macro name clash with MutexLock by renaming the MutexLock class to CeresMutexLock. Change-Id: I923f4427d5939823ea67d48005a90391736d7751 --- CMakeLists.txt | 2 +- internal/ceres/block_random_access_matrix.h | 2 +- internal/ceres/mutex.h | 98 ++++++++++++--------- internal/ceres/schur_eliminator_impl.h | 14 +-- 4 files changed, 63 insertions(+), 53 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 49a139925..628cab747 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -410,7 +410,7 @@ ENDIF ("${UNIX}" AND NOT ${BUILD_ANDROID}) # Disable threads in mutex.h. Someday, after there is OpenMP support in # Android, this can get removed. IF (${BUILD_ANDROID}) - ADD_DEFINITIONS(-DNO_THREADS) + ADD_DEFINITIONS(-DCERES_NO_THREADS) ENDIF (${BUILD_ANDROID}) # Use the std namespace for the hash<> and related templates. This may vary by diff --git a/internal/ceres/block_random_access_matrix.h b/internal/ceres/block_random_access_matrix.h index f398af3be..b76cb78b1 100644 --- a/internal/ceres/block_random_access_matrix.h +++ b/internal/ceres/block_random_access_matrix.h @@ -77,7 +77,7 @@ namespace internal { // // if (cell != NULL) { // MatrixRef m(cell->values, row_stride, col_stride); -// MutexLock l(&cell->m); +// CeresMutexLock l(&cell->m); // m.block(row, col, row_block_size, col_block_size) = ... // } diff --git a/internal/ceres/mutex.h b/internal/ceres/mutex.h index 6514b1070..5090a71b7 100644 --- a/internal/ceres/mutex.h +++ b/internal/ceres/mutex.h @@ -95,11 +95,11 @@ #ifndef CERES_INTERNAL_MUTEX_H_ #define CERES_INTERNAL_MUTEX_H_ -#if defined(NO_THREADS) +#if defined(CERES_NO_THREADS) typedef int MutexType; // to keep a lock-count #elif defined(_WIN32) || defined(__CYGWIN32__) || defined(__CYGWIN64__) -# define WIN32_LEAN_AND_MEAN // We only need minimal includes -# ifdef GMUTEX_TRYLOCK +# define CERES_WIN32_LEAN_AND_MEAN // We only need minimal includes +# ifdef CERES_GMUTEX_TRYLOCK // We need Windows NT or later for TryEnterCriticalSection(). If you // don't need that functionality, you can remove these _WIN32_WINNT // lines, and change TryLock() to assert(0) or something. @@ -108,9 +108,9 @@ # endif # endif // To avoid macro definition of ERROR. -# define NOGDI +# define CERES_NOGDI // To avoid macro definition of min/max. -# define NOMINMAX +# define CERES_NOMINMAX # include typedef CRITICAL_SECTION MutexType; #elif defined(CERES_HAVE_PTHREAD) && defined(CERES_HAVE_RWLOCK) @@ -151,7 +151,7 @@ class Mutex { inline void Lock(); // Block if needed until free then acquire exclusively inline void Unlock(); // Release a lock acquired via Lock() -#ifdef GMUTEX_TRYLOCK +#ifdef CERES_GMUTEX_TRYLOCK inline bool TryLock(); // If free, Lock() and return true, else return false #endif // Note that on systems that don't support read-write locks, these may @@ -183,7 +183,7 @@ class Mutex { }; // Now the implementation of Mutex for various systems -#if defined(NO_THREADS) +#if defined(CERES_NO_THREADS) // When we don't have threads, we can be either reading or writing, // but not both. We can have lots of readers at once (in no-threads @@ -199,7 +199,7 @@ Mutex::Mutex() : mutex_(0) { } Mutex::~Mutex() { assert(mutex_ == 0); } void Mutex::Lock() { assert(--mutex_ == -1); } void Mutex::Unlock() { assert(mutex_++ == -1); } -#ifdef GMUTEX_TRYLOCK +#ifdef CERES_GMUTEX_TRYLOCK bool Mutex::TryLock() { if (mutex_) return false; Lock(); return true; } #endif void Mutex::ReaderLock() { assert(++mutex_ > 0); } @@ -220,91 +220,101 @@ void Mutex::ReaderUnlock() { Unlock(); } #elif defined(CERES_HAVE_PTHREAD) && defined(CERES_HAVE_RWLOCK) -#define SAFE_PTHREAD(fncall) do { /* run fncall if is_safe_ is true */ \ - if (is_safe_ && fncall(&mutex_) != 0) abort(); \ +#define CERES_SAFE_PTHREAD(fncall) do { /* run fncall if is_safe_ is true */ \ + if (is_safe_ && fncall(&mutex_) != 0) abort(); \ } while (0) Mutex::Mutex() { SetIsSafe(); if (is_safe_ && pthread_rwlock_init(&mutex_, NULL) != 0) abort(); } -Mutex::~Mutex() { SAFE_PTHREAD(pthread_rwlock_destroy); } -void Mutex::Lock() { SAFE_PTHREAD(pthread_rwlock_wrlock); } -void Mutex::Unlock() { SAFE_PTHREAD(pthread_rwlock_unlock); } -#ifdef GMUTEX_TRYLOCK +Mutex::~Mutex() { CERES_SAFE_PTHREAD(pthread_rwlock_destroy); } +void Mutex::Lock() { CERES_SAFE_PTHREAD(pthread_rwlock_wrlock); } +void Mutex::Unlock() { CERES_SAFE_PTHREAD(pthread_rwlock_unlock); } +#ifdef CERES_GMUTEX_TRYLOCK bool Mutex::TryLock() { return is_safe_ ? pthread_rwlock_trywrlock(&mutex_) == 0 : true; } #endif -void Mutex::ReaderLock() { SAFE_PTHREAD(pthread_rwlock_rdlock); } -void Mutex::ReaderUnlock() { SAFE_PTHREAD(pthread_rwlock_unlock); } -#undef SAFE_PTHREAD +void Mutex::ReaderLock() { CERES_SAFE_PTHREAD(pthread_rwlock_rdlock); } +void Mutex::ReaderUnlock() { CERES_SAFE_PTHREAD(pthread_rwlock_unlock); } +#undef CERES_SAFE_PTHREAD #elif defined(CERES_HAVE_PTHREAD) -#define SAFE_PTHREAD(fncall) do { /* run fncall if is_safe_ is true */ \ - if (is_safe_ && fncall(&mutex_) != 0) abort(); \ +#define CERES_SAFE_PTHREAD(fncall) do { /* run fncall if is_safe_ is true */ \ + if (is_safe_ && fncall(&mutex_) != 0) abort(); \ } while (0) Mutex::Mutex() { SetIsSafe(); if (is_safe_ && pthread_mutex_init(&mutex_, NULL) != 0) abort(); } -Mutex::~Mutex() { SAFE_PTHREAD(pthread_mutex_destroy); } -void Mutex::Lock() { SAFE_PTHREAD(pthread_mutex_lock); } -void Mutex::Unlock() { SAFE_PTHREAD(pthread_mutex_unlock); } -#ifdef GMUTEX_TRYLOCK +Mutex::~Mutex() { CERES_SAFE_PTHREAD(pthread_mutex_destroy); } +void Mutex::Lock() { CERES_SAFE_PTHREAD(pthread_mutex_lock); } +void Mutex::Unlock() { CERES_SAFE_PTHREAD(pthread_mutex_unlock); } +#ifdef CERES_GMUTEX_TRYLOCK bool Mutex::TryLock() { return is_safe_ ? pthread_mutex_trylock(&mutex_) == 0 : true; } #endif void Mutex::ReaderLock() { Lock(); } void Mutex::ReaderUnlock() { Unlock(); } -#undef SAFE_PTHREAD +#undef CERES_SAFE_PTHREAD #endif // -------------------------------------------------------------------------- // Some helper classes -// MutexLock(mu) acquires mu when constructed and releases it when destroyed. -class MutexLock { +// Note: The weird "Ceres" prefix for the class is a workaround for having two +// similar mutex.h files included in the same translation unit. This is a +// problem because macros do not respect C++ namespaces, and as a result, this +// does not work well (e.g. inside Chrome). The offending macros are +// "MutexLock(x) COMPILE_ASSERT(false)". To work around this, "Ceres" is +// prefixed to the class names; this permits defining the classes. + +// CeresMutexLock(mu) acquires mu when constructed and releases it when destroyed. +class CeresMutexLock { public: - explicit MutexLock(Mutex *mu) : mu_(mu) { mu_->Lock(); } - ~MutexLock() { mu_->Unlock(); } + explicit CeresMutexLock(Mutex *mu) : mu_(mu) { mu_->Lock(); } + ~CeresMutexLock() { mu_->Unlock(); } private: Mutex * const mu_; // Disallow "evil" constructors - MutexLock(const MutexLock&); - void operator=(const MutexLock&); + CeresMutexLock(const CeresMutexLock&); + void operator=(const CeresMutexLock&); }; -// ReaderMutexLock and WriterMutexLock do the same, for rwlocks -class ReaderMutexLock { +// CeresReaderMutexLock and CeresWriterMutexLock do the same, for rwlocks +class CeresReaderMutexLock { public: - explicit ReaderMutexLock(Mutex *mu) : mu_(mu) { mu_->ReaderLock(); } - ~ReaderMutexLock() { mu_->ReaderUnlock(); } + explicit CeresReaderMutexLock(Mutex *mu) : mu_(mu) { mu_->ReaderLock(); } + ~CeresReaderMutexLock() { mu_->ReaderUnlock(); } private: Mutex * const mu_; // Disallow "evil" constructors - ReaderMutexLock(const ReaderMutexLock&); - void operator=(const ReaderMutexLock&); + CeresReaderMutexLock(const CeresReaderMutexLock&); + void operator=(const CeresReaderMutexLock&); }; -class WriterMutexLock { +class CeresWriterMutexLock { public: - explicit WriterMutexLock(Mutex *mu) : mu_(mu) { mu_->WriterLock(); } - ~WriterMutexLock() { mu_->WriterUnlock(); } + explicit CeresWriterMutexLock(Mutex *mu) : mu_(mu) { mu_->WriterLock(); } + ~CeresWriterMutexLock() { mu_->WriterUnlock(); } private: Mutex * const mu_; // Disallow "evil" constructors - WriterMutexLock(const WriterMutexLock&); - void operator=(const WriterMutexLock&); + CeresWriterMutexLock(const CeresWriterMutexLock&); + void operator=(const CeresWriterMutexLock&); }; // Catch bug where variable name is omitted, e.g. MutexLock (&mu); -#define MutexLock(x) COMPILE_ASSERT(0, mutex_lock_decl_missing_var_name) -#define ReaderMutexLock(x) COMPILE_ASSERT(0, rmutex_lock_decl_missing_var_name) -#define WriterMutexLock(x) COMPILE_ASSERT(0, wmutex_lock_decl_missing_var_name) +#define CeresMutexLock(x) \ + COMPILE_ASSERT(0, ceres_mutex_lock_decl_missing_var_name) +#define CeresReaderMutexLock(x) \ + COMPILE_ASSERT(0, ceres_rmutex_lock_decl_missing_var_name) +#define CeresWriterMutexLock(x) \ + COMPILE_ASSERT(0, ceres_wmutex_lock_decl_missing_var_name) } // namespace internal } // namespace ceres diff --git a/internal/ceres/schur_eliminator_impl.h b/internal/ceres/schur_eliminator_impl.h index a388d0054..a916de089 100644 --- a/internal/ceres/schur_eliminator_impl.h +++ b/internal/ceres/schur_eliminator_impl.h @@ -188,7 +188,7 @@ Eliminate(const BlockSparseMatrixBase* A, typename EigenTypes::ConstVectorRef diag(D + bs->cols[i].position, block_size); - MutexLock l(&cell_info->m); + CeresMutexLock l(&cell_info->m); MatrixRef m(cell_info->values, row_stride, col_stride); m.block(r, c, block_size, block_size).diagonal() += diag.array().square().matrix(); @@ -387,7 +387,7 @@ UpdateRhs(const Chunk& chunk, row.block.size, block_size); const int block = block_id - num_eliminate_blocks_; - MutexLock l(rhs_locks_[block]); + CeresMutexLock l(rhs_locks_[block]); typename EigenTypes::VectorRef (rhs + lhs_row_layout_[block], block_size).noalias() += b.transpose() * sj; @@ -523,7 +523,7 @@ ChunkOuterProduct(const CompressedRowBlockStructure* bs, const typename EigenTypes::ConstMatrixRef b2(buffer + it2->second, e_block_size, block2_size); - MutexLock l(&cell_info->m); + CeresMutexLock l(&cell_info->m); MatrixRef m(cell_info->values, row_stride, col_stride); // We explicitly construct a block object here instead of using @@ -601,7 +601,7 @@ NoEBlockRowOuterProduct(const BlockSparseMatrixBase* A, &r, &c, &row_stride, &col_stride); if (cell_info != NULL) { - MutexLock l(&cell_info->m); + CeresMutexLock l(&cell_info->m); MatrixRef m(cell_info->values, row_stride, col_stride); m.block(r, c, block1_size, block1_size) .selfadjointView() @@ -621,7 +621,7 @@ NoEBlockRowOuterProduct(const BlockSparseMatrixBase* A, } const int block2_size = bs->cols[row.cells[j].block_id].size; - MutexLock l(&cell_info->m); + CeresMutexLock l(&cell_info->m); MatrixRef m(cell_info->values, row_stride, col_stride); m.block(r, c, block1_size, block2_size).noalias() += b1.transpose() * ConstMatrixRef(row_values + row.cells[j].position, @@ -660,7 +660,7 @@ EBlockRowOuterProduct(const BlockSparseMatrixBase* A, continue; } - MutexLock l(&cell_info->m); + CeresMutexLock l(&cell_info->m); MatrixRef m(cell_info->values, row_stride, col_stride); Eigen::Block @@ -687,7 +687,7 @@ EBlockRowOuterProduct(const BlockSparseMatrixBase* A, row.block.size, block2_size); - MutexLock l(&cell_info->m); + CeresMutexLock l(&cell_info->m); MatrixRef m(cell_info->values, row_stride, col_stride); Eigen::Block block(m, r, c, block1_size, block2_size);