From 7170bdef2ddb26d6edfbfab5583c60b51969ec74 Mon Sep 17 00:00:00 2001 From: Tobias Leibner <tobias.leibner@googlemail.com> Date: Tue, 13 Jun 2017 16:06:54 +0200 Subject: [PATCH] [container] another attempt on thread-safety The auto& backend_ref = backend(); in many methods has to be protected by the mutex, otherwise the reference may not be valid when it is used. To avoid a double lock, the backend() and ensure_uniqueness() methods now do not lock anymore and thus need to be protected by a mutex if necessary --- dune/xt/la/container/common.hh | 54 +++++++++++++--------------- dune/xt/la/container/eigen/base.hh | 1 + dune/xt/la/container/eigen/dense.hh | 25 ++++++------- dune/xt/la/container/eigen/sparse.hh | 20 +++++------ dune/xt/la/container/istl.hh | 34 +++++++++--------- 5 files changed, 60 insertions(+), 74 deletions(-) diff --git a/dune/xt/la/container/common.hh b/dune/xt/la/container/common.hh index eb142469b..30b48c715 100644 --- a/dune/xt/la/container/common.hh +++ b/dune/xt/la/container/common.hh @@ -163,8 +163,8 @@ public: ThisType& operator=(const ScalarType& value) { - ensure_uniqueness(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + ensure_uniqueness(); for (auto& element : *this) element = value; return *this; @@ -221,15 +221,15 @@ public: void scal(const ScalarType& alpha) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); backend_ref *= alpha; } void axpy(const ScalarType& alpha, const ThisType& xx) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (xx.size() != size()) DUNE_THROW(Common::Exceptions::shapes_do_not_match, "The size of x (" << xx.size() << ") does not match the size of this (" << size() << ")!"); @@ -253,16 +253,16 @@ public: void add_to_entry(const size_t ii, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(ii < size()); backend_ref[ii] += value; } void set_entry(const size_t ii, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(ii < size()); backend_ref[ii] = value; } @@ -328,8 +328,8 @@ public: virtual void iadd(const ThisType& other) override final { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (other.size() != size()) DUNE_THROW(Common::Exceptions::shapes_do_not_match, "The size of other (" << other.size() << ") does not match the size of this (" << size() << ")!"); @@ -338,8 +338,8 @@ public: virtual void isub(const ThisType& other) override final { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (other.size() != size()) DUNE_THROW(Common::Exceptions::shapes_do_not_match, "The size of other (" << other.size() << ") does not match the size of this (" << size() << ")!"); @@ -352,10 +352,8 @@ protected: */ inline void ensure_uniqueness() { - if (!backend_.unique()) { - std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + if (!backend_.unique()) backend_ = std::make_shared<BackendType>(*backend_); - } } // ... ensure_uniqueness(...) private: @@ -478,15 +476,15 @@ public: void scal(const ScalarType& alpha) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); backend_ref *= alpha; } void axpy(const ScalarType& alpha, const ThisType& xx) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (!has_equal_shape(xx)) DUNE_THROW(Common::Exceptions::shapes_do_not_match, "The shape of xx (" << xx.rows() << "x" << xx.cols() << ") does not match the shape of this (" @@ -530,8 +528,8 @@ public: void add_to_entry(const size_t ii, const size_t jj, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(ii < rows()); assert(jj < cols()); backend_ref[ii][jj] += value; @@ -539,8 +537,8 @@ public: void set_entry(const size_t ii, const size_t jj, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(ii < rows()); assert(jj < cols()); backend_ref[ii][jj] = value; @@ -562,8 +560,8 @@ public: void clear_row(const size_t ii) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (ii >= rows()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given ii (" << ii << ") is larger than the rows of this (" << rows() << ")!"); @@ -572,8 +570,8 @@ public: void clear_col(const size_t jj) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (jj >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given jj (" << jj << ") is larger than the cols of this (" << cols() << ")!"); @@ -583,8 +581,8 @@ public: void unit_row(const size_t ii) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (ii >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given ii (" << ii << ") is larger than the cols of this (" << cols() << ")!"); @@ -599,8 +597,8 @@ public: void unit_col(const size_t jj) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (jj >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given jj (" << jj << ") is larger than the cols of this (" << cols() << ")!"); @@ -634,10 +632,8 @@ protected: */ inline void ensure_uniqueness() { - if (!backend_.unique()) { - std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + if (!backend_.unique()) backend_ = std::make_shared<BackendType>(*backend_); - } } // ... ensure_uniqueness(...) private: @@ -788,16 +784,16 @@ public: inline void scal(const ScalarType& alpha) { - ensure_uniqueness(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + ensure_uniqueness(); std::transform( entries_->begin(), entries_->end(), entries_->begin(), std::bind1st(std::multiplies<ScalarType>(), alpha)); } inline void axpy(const ScalarType& alpha, const ThisType& xx) { - ensure_uniqueness(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + ensure_uniqueness(); assert(has_equal_shape(xx)); const auto& xx_entries = *xx.entries_; for (size_t ii = 0; ii < entries_->size(); ++ii) @@ -835,8 +831,8 @@ public: inline void add_to_entry(const size_t rr, const size_t cc, const ScalarType& value) { - ensure_uniqueness(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + ensure_uniqueness(); entries_->operator[](get_entry_index(rr, cc)) += value; } @@ -858,15 +854,15 @@ public: inline void set_entry(const size_t rr, const size_t cc, const ScalarType value) { - ensure_uniqueness(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + ensure_uniqueness(); entries_->operator[](get_entry_index(rr, cc)) = value; } inline void clear_row(const size_t rr) { - ensure_uniqueness(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + ensure_uniqueness(); std::fill(entries_->begin() + row_pointers_->operator[](rr), entries_->begin() + row_pointers_->operator[](rr + 1), ScalarType(0)); @@ -874,8 +870,8 @@ public: inline void clear_col(const size_t cc) { - ensure_uniqueness(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + ensure_uniqueness(); for (size_t kk = 0; kk < entries_->size(); ++kk) { if (column_indices_->operator[](kk) == cc) entries_->operator[](kk) = ScalarType(0); @@ -947,10 +943,8 @@ private: protected: inline void ensure_uniqueness() { - if (!entries_.unique()) { - std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + if (!entries_.unique()) entries_ = std::make_shared<EntriesVectorType>(*entries_); - } } // ... ensure_uniqueness(...) private: diff --git a/dune/xt/la/container/eigen/base.hh b/dune/xt/la/container/eigen/base.hh index 01fa266be..85a773410 100644 --- a/dune/xt/la/container/eigen/base.hh +++ b/dune/xt/la/container/eigen/base.hh @@ -80,6 +80,7 @@ public: VectorImpType& operator=(const ScalarType& value) { + std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); ensure_uniqueness(); for (auto& element : *this) element = value; diff --git a/dune/xt/la/container/eigen/dense.hh b/dune/xt/la/container/eigen/dense.hh index 586325bdd..d105b7858 100644 --- a/dune/xt/la/container/eigen/dense.hh +++ b/dune/xt/la/container/eigen/dense.hh @@ -218,10 +218,8 @@ private: protected: inline void ensure_uniqueness() { - if (!backend_.unique()) { - std::lock_guard<std::mutex> DUNE_UNUSED(lock)(this->mutex_); + if (!backend_.unique()) backend_ = std::make_shared<BackendType>(*(backend_)); - } } // ... ensure_uniqueness(...) private: @@ -342,7 +340,6 @@ protected: inline void ensure_uniqueness() { if (!backend_.unique()) { - std::lock_guard<std::mutex> DUNE_UNUSED(lock)(this->mutex_); auto new_backend = std::make_shared<BackendType>(new ScalarType[backend_->size()], backend_->size()); new_backend->operator=(*(backend_)); backend_ = new_backend; @@ -496,15 +493,15 @@ public: void scal(const ScalarType& alpha) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); backend_ref *= alpha; } void axpy(const ScalarType& alpha, const ThisType& xx) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (!has_equal_shape(xx)) DUNE_THROW(Common::Exceptions::shapes_do_not_match, "The shape of xx (" << xx.rows() << "x" << xx.cols() << ") does not match the shape of this (" @@ -543,8 +540,8 @@ public: void add_to_entry(const size_t ii, const size_t jj, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(ii < rows()); assert(jj < cols()); backend_ref(ii, jj) += value; @@ -552,8 +549,8 @@ public: void set_entry(const size_t ii, const size_t jj, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(ii < rows()); assert(jj < cols()); backend_ref(ii, jj) = value; @@ -575,8 +572,8 @@ public: void clear_row(const size_t ii) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (ii >= rows()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given ii (" << ii << ") is larger than the rows of this (" << rows() << ")!"); @@ -586,8 +583,8 @@ public: void clear_col(const size_t jj) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (jj >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given jj (" << jj << ") is larger than the cols of this (" << cols() << ")!"); @@ -597,8 +594,8 @@ public: void unit_row(const size_t ii) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (ii >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given ii (" << ii << ") is larger than the cols of this (" << cols() << ")!"); @@ -612,8 +609,8 @@ public: void unit_col(const size_t jj) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (jj >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given jj (" << jj << ") is larger than the cols of this (" << cols() << ")!"); @@ -645,10 +642,8 @@ public: protected: inline void ensure_uniqueness() { - if (!backend_.unique()) { - std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + if (!backend_.unique()) backend_ = std::make_shared<BackendType>(*backend_); - } } // ... ensure_uniqueness(...) private: diff --git a/dune/xt/la/container/eigen/sparse.hh b/dune/xt/la/container/eigen/sparse.hh index a35fe5be3..2fb11437a 100644 --- a/dune/xt/la/container/eigen/sparse.hh +++ b/dune/xt/la/container/eigen/sparse.hh @@ -220,15 +220,15 @@ public: void scal(const ScalarType& alpha) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); backend_ref *= alpha; } void axpy(const ScalarType& alpha, const ThisType& xx) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (!has_equal_shape(xx)) DUNE_THROW(Common::Exceptions::shapes_do_not_match, "The shape of xx (" << xx.rows() << "x" << xx.cols() << ") does not match the shape of this (" @@ -267,8 +267,8 @@ public: void add_to_entry(const size_t ii, const size_t jj, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(these_are_valid_indices(ii, jj)); backend_ref.coeffRef(internal::boost_numeric_cast<EIGEN_size_t>(ii), internal::boost_numeric_cast<EIGEN_size_t>(jj)) += value; @@ -276,8 +276,8 @@ public: void set_entry(const size_t ii, const size_t jj, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(these_are_valid_indices(ii, jj)); backend_ref.coeffRef(internal::boost_numeric_cast<EIGEN_size_t>(ii), internal::boost_numeric_cast<EIGEN_size_t>(jj)) = value; @@ -303,8 +303,8 @@ public: void clear_row(const size_t ii) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (ii >= rows()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given ii (" << ii << ") is larger than the rows of this (" << rows() << ")!"); @@ -313,8 +313,8 @@ public: void clear_col(const size_t jj) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (jj >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given jj (" << jj << ") is larger than the cols of this (" << cols() << ")!"); @@ -335,8 +335,8 @@ public: void unit_row(const size_t ii) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (ii >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given ii (" << ii << ") is larger than the cols of this (" << cols() << ")!"); @@ -353,8 +353,8 @@ public: void unit_col(const size_t jj) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (jj >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given jj (" << jj << ") is larger than the cols of this (" << cols() << ")!"); @@ -458,10 +458,8 @@ private: protected: inline void ensure_uniqueness() { - if (!backend_.unique()) { - std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + if (!backend_.unique()) backend_ = std::make_shared<BackendType>(*backend_); - } } // ... ensure_uniqueness(...) private: diff --git a/dune/xt/la/container/istl.hh b/dune/xt/la/container/istl.hh index 058e9fae1..49ff498c0 100644 --- a/dune/xt/la/container/istl.hh +++ b/dune/xt/la/container/istl.hh @@ -210,15 +210,15 @@ public: void scal(const ScalarType& alpha) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); backend_ref *= alpha; } void axpy(const ScalarType& alpha, const ThisType& xx) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (xx.size() != size()) DUNE_THROW(Common::Exceptions::shapes_do_not_match, "The size of x (" << xx.size() << ") does not match the size of this (" << size() << ")!"); @@ -244,16 +244,16 @@ public: void add_to_entry(const size_t ii, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(ii < size()); backend_ref[ii][0] += value; } void set_entry(const size_t ii, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(ii < size()); backend_ref[ii][0] = value; } @@ -319,8 +319,8 @@ public: virtual void iadd(const ThisType& other) override final { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (other.size() != size()) DUNE_THROW(Common::Exceptions::shapes_do_not_match, "The size of other (" << other.size() << ") does not match the size of this (" << size() << ")!"); @@ -329,8 +329,8 @@ public: virtual void isub(const ThisType& other) override final { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (other.size() != size()) DUNE_THROW(Common::Exceptions::shapes_do_not_match, "The size of other (" << other.size() << ") does not match the size of this (" << size() << ")!"); @@ -345,10 +345,8 @@ protected: */ inline void ensure_uniqueness() { - if (!backend_.unique()) { - std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + if (!backend_.unique()) backend_ = std::make_shared<BackendType>(*backend_); - } } // ... ensure_uniqueness(...) private: @@ -478,15 +476,15 @@ public: void scal(const ScalarType& alpha) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); backend_ref *= alpha; } void axpy(const ScalarType& alpha, const ThisType& xx) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (!has_equal_shape(xx)) DUNE_THROW(Common::Exceptions::shapes_do_not_match, "The shape of xx (" << xx.rows() << "x" << xx.cols() << ") does not match the shape of this (" @@ -518,23 +516,23 @@ public: inline void mv(const IstlDenseVector<ScalarType>& xx, IstlDenseVector<ScalarType>& yy) const { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); backend_ref.mv(xx.backend(), yy.backend()); } void add_to_entry(const size_t ii, const size_t jj, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(these_are_valid_indices(ii, jj)); backend_ref[ii][jj][0][0] += value; } void set_entry(const size_t ii, const size_t jj, const ScalarType& value) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); assert(these_are_valid_indices(ii, jj)); backend_ref[ii][jj][0][0] = value; } @@ -560,8 +558,8 @@ public: void clear_row(const size_t ii) { - auto& backend_ref = backend(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + auto& backend_ref = backend(); if (ii >= rows()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given ii (" << ii << ") is larger than the rows of this (" << rows() << ")!"); @@ -570,8 +568,8 @@ public: void clear_col(const size_t jj) { - ensure_uniqueness(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + ensure_uniqueness(); if (jj >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given jj (" << jj << ") is larger than the cols of this (" << cols() << ")!"); @@ -585,8 +583,8 @@ public: void unit_row(const size_t ii) { - ensure_uniqueness(); std::lock_guard<std::mutex> DUNE_UNUSED(lock)(mutex_); + ensure_uniqueness(); if (ii >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, "Given ii (" << ii << ") is larger than the cols of this (" << cols() << ")!"); @@ -602,6 +600,7 @@ public: void unit_col(const size_t jj) { + mutex_.lock(); ensure_uniqueness(); if (jj >= cols()) DUNE_THROW(Common::Exceptions::index_out_of_range, @@ -612,7 +611,6 @@ public: if (!backend_->exists(jj, jj)) DUNE_THROW(Common::Exceptions::index_out_of_range, "Diagonal entry (" << jj << ", " << jj << ") is not contained in the sparsity pattern!"); - mutex_.lock(); for (size_t ii = 0; (ii < rows()) && (ii != jj); ++ii) { auto& row = backend_->operator[](ii); const auto search_result = row.find(jj); -- GitLab