* debug container mutex association
@ 2016-09-13 20:37 François Dumont
2016-09-14 9:01 ` Jonathan Wakely
0 siblings, 1 reply; 16+ messages in thread
From: François Dumont @ 2016-09-13 20:37 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 4267 bytes --]
Hi
When I proposed to change std::hash for pointers my plan was to use
this approach for the debug containers. So here is the patch leveraging
on this technique to avoid going through _Hash_impl to hash address and
get mutex index from it. I think it is obious that new access is faster
so I didn't wrote a performance test to demonstrate it.
Drawback is new exported symbols.
* include/debug/safe_base.h (_Lowest_power_of_two<>): New.
(_Safe_sequence_base): Make _Safe_iterator_base friend.
(_Safe_sequence_base::_M_detach_singular(size_t)): New.
(_Safe_sequence_base::_M_revalidate_singular(size_t)): New.
(_Safe_sequence_base::_M_swap(_Safe_sequence_base&, size_t)): New.
(_Safe_sequence_base::_M_get_mutex(size_t)): New.
(_Safe_iterator_base::_M_get_mutex(size_t)): New.
(_Safe_iterator_base::_M_attach(_Safe_sequence_base*, bool, size_t)):
New.
(_Safe_iterator_base::_M_detach(size_t)): New.
(_Safe_iterator_base(const _Safe_sequence_base*, bool)): Replace by...
(_Safe_iterator_base(const _Safe_sequence_base*, bool, size_t)):
...this.
(_Safe_iterator_base(const _Safe_iterator_base&, bool)): Replace by...
(_Safe_iterator_base(const _Safe_iterator_base&, bool, size_t)):
...this.
(~_Safe_iterator_base()): Delete.
* include/debug/bitset (bitset::_S_alignment()): New.
(bitset::reference::~reference()): New.
(bitset::reference::reference(const _Base_ref&, bitset*)): Adapt.
(bitset::reference::reference(const reference&)): Adapt.
* include/debug/safe_iterator.h (_Safe_iterator::_M_get_mutex()): New.
(_Safe_iterator::_Safe_iterator(const _Iterator&,
const _Safe_sequence_base*)): Adapt.
(_Safe_iterator::~_Safe_iterator()): New.
(_Safe_iterator::_M_detach()): New.
(_Safe_iterator::_M_attach(_Safe_sequence_base*)): Adapt.
* include/debug/safe_iterator.tcc: Remove trailing line.
* include/debug/safe_local_iterator.h (_Safe_local_iterator<>): Rename
_Sequence template parameter into _Container.
(_Safe_local_iterator::_M_get_mutex()): New.
(_Safe_local_iterator::_Safe_local_iterator(const _Iterator&,
const _Safe_sequence_base*)): Adapt.
(_Safe_local_iterator::~_Safe_local_iterator()): New.
(_Safe_local_iterator::_M_detach()): New.
(_Safe_local_iterator::_M_attach(_Safe_sequence_base*)): Adapt.
* include/debug/safe_sequence.h
(_Safe_sequence::_S_alignment()): New.
(_Safe_sequence::_Safe_sequence(_Safe_sequence&&)): New.
(_Safe_sequence::_M_get_mutex()): New.
(_Safe_sequence::_M_detach_singular()): New.
(_Safe_sequence::_M_revalidate_singular()): New.
(_Safe_sequence::_M_swap(_Safe_sequence&)): New.
* include/debug/safe_unordered_base.h
(_Safe_local_iterator_base(const _Safe_sequence_base*, bool)): Replace
by...
(_Safe_local_iterator_base(const _Safe_sequence_base*, bool, size_t)):
...this.
(_Safe_local_iterator_base(const _Safe_local_iterator&, bool)): Replace
by...
(_Safe_local_iterator_base(const _Safe_local_iterator&, bool, size_t)):
...this.
(_Safe_local_iterator_base::~_Safe_local_iterator_base()): Delete.
(_Safe_local_iterator_base::_M_attach(_Safe_sequence_base*, bool,
size_t)): New.
(_Safe_local_iterator_base::_M_detach(size_t)): New.
(_Safe_unordered_container_base(_Safe_unordered_container_base&&)):
Delete.
(_Safe_unordered_container_base::_M_swap(
_Safe_unordered_container_base&, size_t)): New.
(_Safe_unordered_container_base::_M_attach_local(_Safe_iterator_base*,
bool, size_t)): New.
(_Safe_unordered_container_base::_M_detach_local(_Safe_iterator_base*,
size_t)): New.
* include/debug/safe_unordered_container.h
(_Safe_unordered_container::_S_alignment()): New.
(_Safe_unordered_container::_Safe_unordered_container(
_Safe_unordered_container&&)): New.
(_Safe_unordered_container::_M_get_mutex()): New.
(_Safe_unordered_container::_M_swap(_Safe_unordered_container&)): New.
* src/c++11/debug.cc: Adapt.
* testsuite/23_containers/vector/debug/mutex_association.cc: New.
* config/abi/pre/gnu.ver: Add new symbols.
Tested under Linux x86_64.
François
[-- Attachment #2: debug_mutex.patch --]
[-- Type: text/x-patch, Size: 44407 bytes --]
diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index 9b5bb23..c9a253e 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1947,6 +1947,21 @@ GLIBCXX_3.4.23 {
_ZNSsC[12]ERKSs[jmy]RKSaIcE;
_ZNSbIwSt11char_traitsIwESaIwEEC[12]ERKS2_mRKS1_;
+ # __gnu_debug::_Safe_sequence_base
+ _ZN11__gnu_debug19_Safe_sequence_base18_M_detach_singularEm;
+ _ZN11__gnu_debug19_Safe_sequence_base22_M_revalidate_singularEm;
+ _ZN11__gnu_debug19_Safe_sequence_base12_M_get_mutexEm;
+ _ZN11__gnu_debug19_Safe_sequence_base7_M_swapERS0_m;
+
+ # __gnu_debug::_Safe_iterator_base
+ _ZN11__gnu_debug19_Safe_iterator_base9_M_attachEPNS_19_Safe_sequence_baseEbm;
+ _ZN11__gnu_debug19_Safe_iterator_base9_M_detachEm;
+
+ # __gnu_debug::_Safe_unordered_container_base and _Safe_local_iterator_base
+ _ZN11__gnu_debug30_Safe_unordered_container_base7_M_swapERS0_m;
+ _ZN11__gnu_debug25_Safe_local_iterator_base9_M_attachEPNS_19_Safe_sequence_baseEbm;
+ _ZN11__gnu_debug25_Safe_local_iterator_base9_M_detachEm;
+
} GLIBCXX_3.4.22;
# Symbols in the support library (libsupc++) have their own tag.
diff --git a/libstdc++-v3/include/debug/bitset b/libstdc++-v3/include/debug/bitset
index 55d3281..024cc18 100644
--- a/libstdc++-v3/include/debug/bitset
+++ b/libstdc++-v3/include/debug/bitset
@@ -56,6 +56,10 @@ namespace __debug
#if __cplusplus >= 201103L
typedef typename _Base::reference reference;
#else
+ static std::size_t
+ _S_alignment()
+ { return _Lowest_power_of_two<__alignof(bitset)>::__val; }
+
// bit reference:
class reference
: private _Base::reference
@@ -66,22 +70,24 @@ namespace __debug
friend class bitset;
reference();
- reference(const _Base_ref& __base,
- bitset* __seq __attribute__((__unused__))) _GLIBCXX_NOEXCEPT
+ reference(const _Base_ref& __base, bitset* __seq) _GLIBCXX_NOEXCEPT
: _Base_ref(__base)
- , _Safe_iterator_base(__seq, false)
+ , _Safe_iterator_base(__seq, false, _S_alignment())
{ }
public:
reference(const reference& __x) _GLIBCXX_NOEXCEPT
: _Base_ref(__x)
- , _Safe_iterator_base(__x, false)
+ , _Safe_iterator_base(__x, false, _S_alignment())
{ }
+ ~reference()
+ { this->_M_detach(_S_alignment()); }
+
reference&
operator=(bool __x) _GLIBCXX_NOEXCEPT
{
- _GLIBCXX_DEBUG_VERIFY(! this->_M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_write)
._M_iterator(*this));
*static_cast<_Base_ref*>(this) = __x;
@@ -91,10 +97,10 @@ namespace __debug
reference&
operator=(const reference& __x) _GLIBCXX_NOEXCEPT
{
- _GLIBCXX_DEBUG_VERIFY(! __x._M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!__x._M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_read)
._M_iterator(__x));
- _GLIBCXX_DEBUG_VERIFY(! this->_M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_write)
._M_iterator(*this));
*static_cast<_Base_ref*>(this) = __x;
@@ -104,7 +110,7 @@ namespace __debug
bool
operator~() const _GLIBCXX_NOEXCEPT
{
- _GLIBCXX_DEBUG_VERIFY(! this->_M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_read)
._M_iterator(*this));
return ~(*static_cast<const _Base_ref*>(this));
@@ -112,7 +118,7 @@ namespace __debug
operator bool() const _GLIBCXX_NOEXCEPT
{
- _GLIBCXX_DEBUG_VERIFY(! this->_M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_read)
._M_iterator(*this));
return *static_cast<const _Base_ref*>(this);
@@ -121,7 +127,7 @@ namespace __debug
reference&
flip() _GLIBCXX_NOEXCEPT
{
- _GLIBCXX_DEBUG_VERIFY(! this->_M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_flip)
._M_iterator(*this));
_Base_ref::flip();
diff --git a/libstdc++-v3/include/debug/safe_base.h b/libstdc++-v3/include/debug/safe_base.h
index aad9c52..7c3e4a0 100644
--- a/libstdc++-v3/include/debug/safe_base.h
+++ b/libstdc++-v3/include/debug/safe_base.h
@@ -49,6 +49,8 @@ namespace __gnu_debug
*/
class _Safe_iterator_base
{
+ friend class _Safe_sequence_base;
+
public:
/** The sequence this iterator references; may be NULL to indicate
a singular iterator. */
@@ -84,31 +86,34 @@ namespace __gnu_debug
* singular. Otherwise, the iterator will reference @p __seq and
* be nonsingular.
*/
- _Safe_iterator_base(const _Safe_sequence_base* __seq, bool __constant)
+ _Safe_iterator_base(const _Safe_sequence_base* __seq, bool __constant,
+ std::size_t __alignment)
: _M_sequence(0), _M_version(0), _M_prior(0), _M_next(0)
- { this->_M_attach(const_cast<_Safe_sequence_base*>(__seq), __constant); }
+ {
+ this->_M_attach(const_cast<_Safe_sequence_base*>(__seq), __constant,
+ __alignment);
+ }
/** Initializes the iterator to reference the same sequence that
@p __x does. @p __constant is true if this is a constant
iterator, and false if it is mutable. */
- _Safe_iterator_base(const _Safe_iterator_base& __x, bool __constant)
+ _Safe_iterator_base(const _Safe_iterator_base& __x, bool __constant,
+ std::size_t __alignment)
: _M_sequence(0), _M_version(0), _M_prior(0), _M_next(0)
- { this->_M_attach(__x._M_sequence, __constant); }
-
- ~_Safe_iterator_base() { this->_M_detach(); }
+ { this->_M_attach(__x._M_sequence, __constant, __alignment); }
/** For use in _Safe_iterator. */
__gnu_cxx::__mutex&
- _M_get_mutex() throw ();
+ _M_get_mutex(std::size_t __alignment) throw ();
- public:
/** Attaches this iterator to the given sequence, detaching it
* from whatever sequence it was attached to originally. If the
* new sequence is the NULL pointer, the iterator is left
* unattached.
*/
void
- _M_attach(_Safe_sequence_base* __seq, bool __constant);
+ _M_attach(_Safe_sequence_base* __seq, bool __constant,
+ std::size_t __alignment);
/** Likewise, but not thread-safe. */
void
@@ -118,12 +123,13 @@ namespace __gnu_debug
* if any.
*/
void
- _M_detach();
+ _M_detach(std::size_t __alignment);
/** Likewise, but not thread-safe. */
void
_M_detach_single() throw ();
+ public:
/** Determines if we are attached to the given sequence. */
bool
_M_attached_to(const _Safe_sequence_base* __seq) const
@@ -157,6 +163,17 @@ namespace __gnu_debug
if (_M_next)
_M_next->_M_prior = _M_prior;
}
+
+ private:
+ /** Methods kept for abi compatibility */
+ __gnu_cxx::__mutex&
+ _M_get_mutex() throw ();
+
+ void
+ _M_attach(_Safe_sequence_base* __seq, bool __constant);
+
+ void
+ _M_detach();
};
/** Iterators that derive from _Safe_iterator_base can be determined singular
@@ -174,6 +191,18 @@ namespace __gnu_debug
const _Safe_iterator_base* __last)
{ return __first->_M_can_compare(*__last); }
+ // Compute power of 2 not greater than __n.
+ template<size_t __n>
+ struct _Lowest_power_of_two
+ {
+ static const size_t __val
+ = _Lowest_power_of_two< (__n >> 1) >::__val + 1;
+ };
+
+ template<>
+ struct _Lowest_power_of_two<1>
+ { static const size_t __val = 1; };
+
/**
* @brief Base class that supports tracking of iterators that
* reference a sequence.
@@ -193,6 +222,8 @@ namespace __gnu_debug
*/
class _Safe_sequence_base
{
+ friend class _Safe_iterator_base;
+
public:
/// The list of mutable iterators that reference this container
_Safe_iterator_base* _M_iterators;
@@ -212,6 +243,8 @@ namespace __gnu_debug
#if __cplusplus >= 201103L
_Safe_sequence_base(const _Safe_sequence_base&) noexcept
: _Safe_sequence_base() { }
+ _Safe_sequence_base(_Safe_sequence_base&&) noexcept
+ : _Safe_sequence_base() { }
#endif
/** Notify all iterators that reference this sequence that the
@@ -228,7 +261,7 @@ namespace __gnu_debug
* i->_M_version == _M_version.
*/
void
- _M_detach_singular();
+ _M_detach_singular(std::size_t __alignment);
/** Revalidates all attached singular iterators. This method may
* be used to validate iterators that were invalidated before
@@ -236,7 +269,7 @@ namespace __gnu_debug
* valid again).
*/
void
- _M_revalidate_singular();
+ _M_revalidate_singular(std::size_t __alignment);
/** Swap this sequence with the given sequence. This operation
* also swaps ownership of the iterators, so that when the
@@ -244,21 +277,35 @@ namespace __gnu_debug
* one container now reference the other container.
*/
void
- _M_swap(_Safe_sequence_base& __x) _GLIBCXX_USE_NOEXCEPT;
+ _M_swap(_Safe_sequence_base& __x,
+ std::size_t __alignment) _GLIBCXX_USE_NOEXCEPT;
/** For use in _Safe_sequence. */
__gnu_cxx::__mutex&
+ _M_get_mutex(std::size_t __alignment) throw ();
+
+ __gnu_cxx::__mutex&
_M_get_mutex() throw ();
- public:
/** Invalidates all iterators. */
void
_M_invalidate_all() const
{ if (++_M_version == 0) _M_version = 1; }
+ private:
+ void
+ _M_detach_singular_single();
+
+ void
+ _M_revalidate_singular_single();
+
+ void
+ _M_attach(_Safe_sequence_base* __seq, bool __constant);
+
/** Attach an iterator to this sequence. */
void
- _M_attach(_Safe_iterator_base* __it, bool __constant);
+ _M_attach(_Safe_iterator_base* __it, bool __constant,
+ std::size_t __alignment);
/** Likewise but not thread safe. */
void
@@ -266,12 +313,33 @@ namespace __gnu_debug
/** Detach an iterator from this sequence */
void
- _M_detach(_Safe_iterator_base* __it);
+ _M_detach(_Safe_iterator_base* __it, std::size_t __alignment);
/** Likewise but not thread safe. */
void
_M_detach_single(_Safe_iterator_base* __it) throw ();
+
+ /** Methods kept for abi compatibility */
+ void
+ _M_detach_singular();
+
+ void
+ _M_revalidate_singular();
+
+ void
+ _M_attach(_Safe_iterator_base* __it, bool __constant);
+
+ void
+ _M_detach(_Safe_iterator_base* __it);
+
+ void
+ _M_swap(_Safe_sequence_base& __x) _GLIBCXX_USE_NOEXCEPT;
};
+
+ inline __gnu_cxx::__mutex&
+ _Safe_iterator_base::_M_get_mutex(std::size_t __alignment) throw ()
+ { return _M_sequence->_M_get_mutex(__alignment); }
+
} // namespace __gnu_debug
#endif
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index c95d36c..865d34e 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -107,6 +107,10 @@ namespace __gnu_debug
: _Iter_base(__i)
{ _M_attach_single(__seq); }
+ __gnu_cxx::__mutex&
+ _M_get_mutex() throw ()
+ { return _Safe_base::_M_get_mutex(_Sequence::_S_alignment()); }
+
public:
typedef _Iterator iterator_type;
typedef typename _Traits::iterator_category iterator_category;
@@ -127,7 +131,8 @@ namespace __gnu_debug
*/
_Safe_iterator(const _Iterator& __i, const _Safe_sequence_base* __seq)
_GLIBCXX_NOEXCEPT
- : _Iter_base(__i), _Safe_base(__seq, _M_constant())
+ : _Iter_base(__i),
+ _Safe_base(__seq, _M_constant(), _Sequence::_S_alignment())
{
_GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__msg_init_singular)
@@ -192,6 +197,9 @@ namespace __gnu_debug
_M_attach(__x._M_sequence);
}
+ ~_Safe_iterator()
+ { _M_detach(); }
+
/**
* @brief Copy assignment.
*/
@@ -295,7 +303,7 @@ namespace __gnu_debug
_GLIBCXX_DEBUG_VERIFY(this->_M_incrementable(),
_M_message(__msg_bad_inc)
._M_iterator(*this, "this"));
- __gnu_cxx::__scoped_lock(this->_M_get_mutex());
+ __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
++base();
return *this;
}
@@ -411,10 +419,18 @@ namespace __gnu_debug
*/
operator _Iterator() const _GLIBCXX_NOEXCEPT { return *this; }
+ /** Detach iterator from its sequence. */
+ void
+ _M_detach()
+ { _Safe_base::_M_detach(_Sequence::_S_alignment()); }
+
/** Attach iterator to the given sequence. */
void
_M_attach(_Safe_sequence_base* __seq)
- { _Safe_base::_M_attach(__seq, _M_constant()); }
+ {
+ _Safe_base::_M_attach(__seq, _M_constant(),
+ _Sequence::_S_alignment());
+ }
/** Likewise, but not thread-safe. */
void
diff --git a/libstdc++-v3/include/debug/safe_iterator.tcc b/libstdc++-v3/include/debug/safe_iterator.tcc
index e8e1b00..0ae7fd1 100644
--- a/libstdc++-v3/include/debug/safe_iterator.tcc
+++ b/libstdc++-v3/include/debug/safe_iterator.tcc
@@ -93,4 +93,3 @@ namespace __gnu_debug
} // namespace __gnu_debug
#endif
-
diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h
index e9ac74e..55d1279 100644
--- a/libstdc++-v3/include/debug/safe_local_iterator.h
+++ b/libstdc++-v3/include/debug/safe_local_iterator.h
@@ -44,15 +44,15 @@ namespace __gnu_debug
* attaching/detaching the iterator from sequences, and querying
* the iterator's state.
*/
- template<typename _Iterator, typename _Sequence>
+ template<typename _Iterator, typename _Container>
class _Safe_local_iterator
: private _Iterator
, public _Safe_local_iterator_base
{
typedef _Iterator _Iter_base;
typedef _Safe_local_iterator_base _Safe_base;
- typedef typename _Sequence::const_local_iterator _Const_local_iterator;
- typedef typename _Sequence::size_type size_type;
+ typedef typename _Container::const_local_iterator _Const_local_iterator;
+ typedef typename _Container::size_type size_type;
/// Determine if this is a constant iterator.
bool
@@ -72,6 +72,10 @@ namespace __gnu_debug
: _Iter_base(__i)
{ _M_attach_single(__cont); }
+ __gnu_cxx::__mutex&
+ _M_get_mutex() throw ()
+ { return _Safe_base::_M_get_mutex(_Container::_S_alignment()); }
+
public:
typedef _Iterator iterator_type;
typedef typename _Traits::iterator_category iterator_category;
@@ -92,7 +96,8 @@ namespace __gnu_debug
*/
_Safe_local_iterator(const _Iterator& __i,
const _Safe_sequence_base* __cont)
- : _Iter_base(__i), _Safe_base(__cont, _M_constant())
+ : _Iter_base(__i),
+ _Safe_base(__cont, _M_constant(), _Container::_S_alignment())
{
_GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__msg_init_singular)
@@ -142,8 +147,8 @@ namespace __gnu_debug
const _Safe_local_iterator<_MutableIterator,
typename __gnu_cxx::__enable_if<std::__are_same<
_MutableIterator,
- typename _Sequence::local_iterator::iterator_type>::__value,
- _Sequence>::__type>& __x)
+ typename _Container::local_iterator::iterator_type>::__value,
+ _Container>::__type>& __x)
: _Iter_base(__x.base())
{
// _GLIBCXX_RESOLVE_LIB_DEFECTS
@@ -156,6 +161,9 @@ namespace __gnu_debug
_M_attach(__x._M_sequence);
}
+ ~_Safe_local_iterator()
+ { _M_detach(); }
+
/**
* @brief Copy assignment.
*/
@@ -299,15 +307,23 @@ namespace __gnu_debug
*/
operator _Iterator() const { return *this; }
+ /** Detach iterator. */
+ void
+ _M_detach()
+ { _Safe_base::_M_detach(_Container::_S_alignment()); }
+
/** Attach iterator to the given sequence. */
void
- _M_attach(_Safe_sequence_base* __seq)
- { _Safe_base::_M_attach(__seq, _M_constant()); }
+ _M_attach(_Safe_sequence_base* __cont)
+ {
+ _Safe_base::_M_attach(__cont, _M_constant(),
+ _Container::_S_alignment());
+ }
/** Likewise, but not thread-safe. */
void
- _M_attach_single(_Safe_sequence_base* __seq)
- { _Safe_base::_M_attach_single(__seq, _M_constant()); }
+ _M_attach_single(_Safe_sequence_base* __cont)
+ { _Safe_base::_M_attach_single(__cont, _M_constant()); }
/// Is the iterator dereferenceable?
bool
@@ -329,10 +345,10 @@ namespace __gnu_debug
typename
__gnu_cxx::__conditional_type<std::__are_same<_Const_local_iterator,
_Safe_local_iterator>::__value,
- const _Sequence*,
- _Sequence*>::__type
+ const _Container*,
+ _Container*>::__type
_M_get_sequence() const
- { return static_cast<_Sequence*>(_M_sequence); }
+ { return static_cast<_Container*>(_M_sequence); }
/// Is this iterator equal to the sequence's begin(bucket) iterator?
bool _M_is_begin() const
@@ -346,14 +362,14 @@ namespace __gnu_debug
template<typename _Other>
bool
_M_in_same_bucket(const _Safe_local_iterator<_Other,
- _Sequence>& __other) const
+ _Container>& __other) const
{ return bucket() == __other.bucket(); }
};
- template<typename _IteratorL, typename _IteratorR, typename _Sequence>
+ template<typename _IteratorL, typename _IteratorR, typename _Container>
inline bool
- operator==(const _Safe_local_iterator<_IteratorL, _Sequence>& __lhs,
- const _Safe_local_iterator<_IteratorR, _Sequence>& __rhs)
+ operator==(const _Safe_local_iterator<_IteratorL, _Container>& __lhs,
+ const _Safe_local_iterator<_IteratorR, _Container>& __rhs)
{
_GLIBCXX_DEBUG_VERIFY(!__lhs._M_singular() && !__rhs._M_singular(),
_M_message(__msg_iter_compare_bad)
@@ -370,10 +386,10 @@ namespace __gnu_debug
return __lhs.base() == __rhs.base();
}
- template<typename _Iterator, typename _Sequence>
+ template<typename _Iterator, typename _Container>
inline bool
- operator==(const _Safe_local_iterator<_Iterator, _Sequence>& __lhs,
- const _Safe_local_iterator<_Iterator, _Sequence>& __rhs)
+ operator==(const _Safe_local_iterator<_Iterator, _Container>& __lhs,
+ const _Safe_local_iterator<_Iterator, _Container>& __rhs)
{
_GLIBCXX_DEBUG_VERIFY(!__lhs._M_singular() && !__rhs._M_singular(),
_M_message(__msg_iter_compare_bad)
@@ -390,10 +406,10 @@ namespace __gnu_debug
return __lhs.base() == __rhs.base();
}
- template<typename _IteratorL, typename _IteratorR, typename _Sequence>
+ template<typename _IteratorL, typename _IteratorR, typename _Container>
inline bool
- operator!=(const _Safe_local_iterator<_IteratorL, _Sequence>& __lhs,
- const _Safe_local_iterator<_IteratorR, _Sequence>& __rhs)
+ operator!=(const _Safe_local_iterator<_IteratorL, _Container>& __lhs,
+ const _Safe_local_iterator<_IteratorR, _Container>& __rhs)
{
_GLIBCXX_DEBUG_VERIFY(! __lhs._M_singular() && ! __rhs._M_singular(),
_M_message(__msg_iter_compare_bad)
@@ -410,10 +426,10 @@ namespace __gnu_debug
return __lhs.base() != __rhs.base();
}
- template<typename _Iterator, typename _Sequence>
+ template<typename _Iterator, typename _Container>
inline bool
- operator!=(const _Safe_local_iterator<_Iterator, _Sequence>& __lhs,
- const _Safe_local_iterator<_Iterator, _Sequence>& __rhs)
+ operator!=(const _Safe_local_iterator<_Iterator, _Container>& __lhs,
+ const _Safe_local_iterator<_Iterator, _Container>& __rhs)
{
_GLIBCXX_DEBUG_VERIFY(!__lhs._M_singular() && !__rhs._M_singular(),
_M_message(__msg_iter_compare_bad)
@@ -431,33 +447,33 @@ namespace __gnu_debug
}
/** Safe local iterators know if they are dereferenceable. */
- template<typename _Iterator, typename _Sequence>
+ template<typename _Iterator, typename _Container>
inline bool
__check_dereferenceable(const _Safe_local_iterator<_Iterator,
- _Sequence>& __x)
+ _Container>& __x)
{ return __x._M_dereferenceable(); }
/** Safe local iterators know how to check if they form a valid range. */
- template<typename _Iterator, typename _Sequence>
+ template<typename _Iterator, typename _Container>
inline bool
- __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>& __first,
- const _Safe_local_iterator<_Iterator, _Sequence>& __last,
+ __valid_range(const _Safe_local_iterator<_Iterator, _Container>& __first,
+ const _Safe_local_iterator<_Iterator, _Container>& __last,
typename _Distance_traits<_Iterator>::__type& __dist)
{ return __first._M_valid_range(__last, __dist); }
/* Safe local iterators know their bucket. */
- template<typename _Iterator, typename _Sequence>
+ template<typename _Iterator, typename _Container>
inline bool
- __same_bucket(const _Safe_local_iterator<_Iterator, _Sequence>& __first,
- const _Safe_local_iterator<_Iterator, _Sequence>& __last)
+ __same_bucket(const _Safe_local_iterator<_Iterator, _Container>& __first,
+ const _Safe_local_iterator<_Iterator, _Container>& __last)
{ return __first._M_in_same_bucket(__last); }
/** Safe local iterators need a special method to get distance between each
other. */
- template<typename _Iterator, typename _Sequence>
+ template<typename _Iterator, typename _Container>
inline typename _Distance_traits<_Iterator>::__type
- __get_distance(const _Safe_local_iterator<_Iterator, _Sequence>& __first,
- const _Safe_local_iterator<_Iterator, _Sequence>& __last,
+ __get_distance(const _Safe_local_iterator<_Iterator, _Container>& __first,
+ const _Safe_local_iterator<_Iterator, _Container>& __last,
std::input_iterator_tag)
{
if (__first.base() == __last.base())
@@ -496,10 +512,10 @@ namespace __gnu_debug
return { 1, __dp_equality };
}
- template<typename _Iterator, typename _Sequence>
+ template<typename _Iterator, typename _Container>
inline typename _Distance_traits<_Iterator>::__type
__get_distance_from_begin(
- const _Safe_local_iterator<_Iterator, _Sequence>& __it)
+ const _Safe_local_iterator<_Iterator, _Container>& __it)
{
if (__it._M_is_begin())
return { 0, __dp_exact };
@@ -511,10 +527,10 @@ namespace __gnu_debug
return { 1, __dp_equality };
}
- template<typename _Iterator, typename _Sequence>
+ template<typename _Iterator, typename _Container>
inline typename _Distance_traits<_Iterator>::__type
__get_distance_to_end(
- const _Safe_local_iterator<_Iterator, _Sequence>& __it)
+ const _Safe_local_iterator<_Iterator, _Container>& __it)
{
if (__it._M_is_begin())
return
@@ -527,23 +543,23 @@ namespace __gnu_debug
}
#if __cplusplus < 201103L
- template<typename _Iterator, typename _Sequence>
- struct _Unsafe_type<_Safe_local_iterator<_Iterator, _Sequence> >
+ template<typename _Iterator, typename _Container>
+ struct _Unsafe_type<_Safe_local_iterator<_Iterator, _Container> >
{ typedef _Iterator _Type; };
#endif
- template<typename _Iterator, typename _Sequence>
+ template<typename _Iterator, typename _Container>
inline _Iterator
- __unsafe(const _Safe_local_iterator<_Iterator, _Sequence>& __it)
+ __unsafe(const _Safe_local_iterator<_Iterator, _Container>& __it)
{ return __it.base(); }
// Conversion to an unordered container safe local iterator.
- template<typename _Iterator, typename _Sequence>
- inline _Safe_local_iterator<_Iterator, _Sequence>
- __safe(const _Safe_local_iterator<_Iterator, _Sequence>& __safe_it,
+ template<typename _Iterator, typename _Container>
+ inline _Safe_local_iterator<_Iterator, _Container>
+ __safe(const _Safe_local_iterator<_Iterator, _Container>& __safe_it,
_Iterator __it)
{
- return _Safe_local_iterator<_Iterator, _Sequence>(__it,
+ return _Safe_local_iterator<_Iterator, _Container>(__it,
__safe_it._M_get_sequence());
}
diff --git a/libstdc++-v3/include/debug/safe_sequence.h b/libstdc++-v3/include/debug/safe_sequence.h
index e12b6f1..73e6ec7 100644
--- a/libstdc++-v3/include/debug/safe_sequence.h
+++ b/libstdc++-v3/include/debug/safe_sequence.h
@@ -107,6 +107,8 @@ namespace __gnu_debug
template<typename _Sequence>
class _Safe_sequence : public _Safe_sequence_base
{
+ typedef _Safe_sequence_base _Base;
+
public:
/** Invalidates all iterators @c x that reference this sequence,
are not singular, and for which @c __pred(x) returns @c
@@ -123,6 +125,36 @@ namespace __gnu_debug
template<typename _Predicate>
void
_M_transfer_from_if(_Safe_sequence& __from, _Predicate __pred);
+
+ static _GLIBCXX_CONSTEXPR std::size_t
+ _S_alignment() _GLIBCXX_NOEXCEPT
+ { return _Lowest_power_of_two<__alignof(_Sequence)>::__val; }
+
+ protected:
+#if __cplusplus >= 201103L
+ _Safe_sequence() = default;
+ _Safe_sequence(const _Safe_sequence&) = default;
+
+ // Move constructor swap iterators.
+ _Safe_sequence(_Safe_sequence&& __seq)
+ { _M_swap(__seq); }
+#endif
+
+ __gnu_cxx::__mutex&
+ _M_get_mutex() throw ()
+ { return _Base::_M_get_mutex(_S_alignment()); }
+
+ void
+ _M_detach_singular()
+ { _Base::_M_detach_singular(_S_alignment()); }
+
+ void
+ _M_revalidate_singular()
+ { _Base::_M_revalidate_singular(_S_alignment()); }
+
+ void
+ _M_swap(_Safe_sequence& __x) _GLIBCXX_USE_NOEXCEPT
+ { _Base::_M_swap(__x, _S_alignment()); }
};
/// Like _Safe_sequence but with a special _M_invalidate_all implementation
diff --git a/libstdc++-v3/include/debug/safe_unordered_base.h b/libstdc++-v3/include/debug/safe_unordered_base.h
index 82a42eb..3d3bab6 100644
--- a/libstdc++-v3/include/debug/safe_unordered_base.h
+++ b/libstdc++-v3/include/debug/safe_unordered_base.h
@@ -61,17 +61,19 @@ namespace __gnu_debug
* singular. Otherwise, the iterator will reference @p __seq and
* be nonsingular.
*/
- _Safe_local_iterator_base(const _Safe_sequence_base* __seq, bool __constant)
- { this->_M_attach(const_cast<_Safe_sequence_base*>(__seq), __constant); }
+ _Safe_local_iterator_base(const _Safe_sequence_base* __seq,
+ bool __constant, std::size_t __alignment)
+ {
+ this->_M_attach(const_cast<_Safe_sequence_base*>(__seq),
+ __constant, __alignment);
+ }
/** Initializes the iterator to reference the same container that
@p __x does. @p __constant is true if this is a constant
iterator, and false if it is mutable. */
_Safe_local_iterator_base(const _Safe_local_iterator_base& __x,
- bool __constant)
- { this->_M_attach(__x._M_sequence, __constant); }
-
- ~_Safe_local_iterator_base() { this->_M_detach(); }
+ bool __constant, std::size_t __alignment)
+ { this->_M_attach(__x._M_sequence, __constant, __alignment); }
_Safe_unordered_container_base*
_M_get_container() const noexcept;
@@ -82,18 +84,30 @@ namespace __gnu_debug
* new container is the NULL pointer, the iterator is left
* unattached.
*/
- void _M_attach(_Safe_sequence_base* __seq, bool __constant);
+ void
+ _M_attach(_Safe_sequence_base* __seq, bool __constant,
+ std::size_t __alignment);
/** Likewise, but not thread-safe. */
- void _M_attach_single(_Safe_sequence_base* __seq, bool __constant) throw ();
+ void
+ _M_attach_single(_Safe_sequence_base* __seq, bool __constant) throw ();
/** Detach the iterator for whatever container it is attached to,
* if any.
*/
- void _M_detach();
+ void
+ _M_detach(std::size_t __alignment);
/** Likewise, but not thread-safe. */
- void _M_detach_single() throw ();
+ void
+ _M_detach_single() throw ();
+
+ private:
+ void
+ _M_attach(_Safe_sequence_base* __seq, bool __constant);
+
+ void
+ _M_detach();
};
/**
@@ -116,7 +130,9 @@ namespace __gnu_debug
*/
class _Safe_unordered_container_base : public _Safe_sequence_base
{
+ friend class _Safe_local_iterator_base;
typedef _Safe_sequence_base _Base;
+
public:
/// The list of mutable local iterators that reference this container
_Safe_iterator_base* _M_local_iterators;
@@ -135,12 +151,6 @@ namespace __gnu_debug
noexcept
: _Safe_unordered_container_base() { }
- // When moved unordered containers iterators are swapped.
- _Safe_unordered_container_base(_Safe_unordered_container_base&& __x)
- noexcept
- : _Safe_unordered_container_base()
- { this->_M_swap(__x); }
-
/** Notify all iterators that reference this container that the
container is being destroyed. */
~_Safe_unordered_container_base() noexcept
@@ -156,11 +166,19 @@ namespace __gnu_debug
* one container now reference the other container.
*/
void
+ _M_swap(_Safe_unordered_container_base& __x,
+ std::size_t __alignment) noexcept;
+
+ private:
+ void
_M_swap(_Safe_unordered_container_base& __x) noexcept;
- public:
/** Attach an iterator to this container. */
void
+ _M_attach_local(_Safe_iterator_base* __it, bool __constant,
+ std::size_t __alignment);
+
+ void
_M_attach_local(_Safe_iterator_base* __it, bool __constant);
/** Likewise but not thread safe. */
@@ -169,6 +187,9 @@ namespace __gnu_debug
/** Detach an iterator from this container */
void
+ _M_detach_local(_Safe_iterator_base* __it, std::size_t __alignment);
+
+ void
_M_detach_local(_Safe_iterator_base* __it);
/** Likewise but not thread safe. */
diff --git a/libstdc++-v3/include/debug/safe_unordered_container.h b/libstdc++-v3/include/debug/safe_unordered_container.h
index 4c4a138..a15677a 100644
--- a/libstdc++-v3/include/debug/safe_unordered_container.h
+++ b/libstdc++-v3/include/debug/safe_unordered_container.h
@@ -58,11 +58,34 @@ namespace __gnu_debug
class _Safe_unordered_container : public _Safe_unordered_container_base
{
private:
+ typedef _Safe_unordered_container_base _Base;
+
_Container&
_M_cont() noexcept
{ return *static_cast<_Container*>(this); }
+ public:
+ static constexpr std::size_t
+ _S_alignment() noexcept
+ { return _Lowest_power_of_two<alignof(_Container)>::__val; }
+
protected:
+ _Safe_unordered_container() = default;
+ _Safe_unordered_container(const _Safe_unordered_container&) = default;
+
+ // Move constructor swap iterators.
+ _Safe_unordered_container(_Safe_unordered_container&& __x) noexcept
+ : _Base(std::move(__x))
+ { _M_swap(__x); }
+
+ __gnu_cxx::__mutex&
+ _M_get_mutex() throw ()
+ { return _Base::_M_get_mutex(_S_alignment()); }
+
+ void
+ _M_swap(_Safe_unordered_container& __x) noexcept
+ { _Base::_M_swap(__x, _S_alignment()); }
+
void
_M_invalidate_locals()
{
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index ed0417a..769904e 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -33,9 +33,10 @@
#include <cassert>
#include <cstdio>
+#include <cctype> // for std::isspace
#include <algorithm> // for std::min
-#include <functional> // for _Hash_impl
+#include <functional> // for _Hash_impl and std::function
#include <cxxabi.h> // for __cxa_demangle
@@ -46,15 +47,30 @@ namespace
/** Returns different instances of __mutex depending on the passed address
* in order to limit contention without breaking current library binary
* compatibility. */
+ const size_t mask = 0xf;
+
__gnu_cxx::__mutex&
- get_safe_base_mutex(void* __address)
+ get_mutex(size_t index)
{
- const size_t mask = 0xf;
static __gnu_cxx::__mutex safe_base_mutex[mask + 1];
- const size_t index = _Hash_impl::hash(__address) & mask;
return safe_base_mutex[index];
}
+ __gnu_cxx::__mutex&
+ get_safe_base_mutex(void* address)
+ {
+ const size_t index = _Hash_impl::hash(address) & mask;
+ return get_mutex(index);
+ }
+
+ __gnu_cxx::__mutex&
+ get_safe_base_mutex(void* address, std::size_t alignment)
+ {
+ const size_t index
+ = (reinterpret_cast<size_t>(address) >> alignment) & mask;
+ return get_mutex(index);
+ }
+
void
swap_its(__gnu_debug::_Safe_sequence_base& __lhs,
__gnu_debug::_Safe_iterator_base*& __lhs_its,
@@ -70,8 +86,8 @@ namespace
}
void
- swap_seq(__gnu_debug::_Safe_sequence_base& __lhs,
- __gnu_debug::_Safe_sequence_base& __rhs)
+ swap_seq_single(__gnu_debug::_Safe_sequence_base& __lhs,
+ __gnu_debug::_Safe_sequence_base& __rhs)
{
swap(__lhs._M_version, __rhs._M_version);
swap_its(__lhs, __lhs._M_iterators,
@@ -81,10 +97,41 @@ namespace
}
void
- swap_ucont(__gnu_debug::_Safe_unordered_container_base& __lhs,
- __gnu_debug::_Safe_unordered_container_base& __rhs)
+ lock_and_run(__gnu_cxx::__mutex& lhs_mutex,
+ __gnu_cxx::__mutex& rhs_mutex,
+ std::function<void()> action)
+ {
+ // We need to lock both sequences to run action.
+ if (&lhs_mutex == &rhs_mutex)
+ {
+ __gnu_cxx::__scoped_lock sentry(lhs_mutex);
+ action();
+ }
+ else
+ {
+ __gnu_cxx::__scoped_lock sentry1(&lhs_mutex < &rhs_mutex
+ ? lhs_mutex : rhs_mutex);
+ __gnu_cxx::__scoped_lock sentry2(&lhs_mutex < &rhs_mutex
+ ? rhs_mutex : lhs_mutex);
+ action();
+ }
+ }
+
+ void
+ swap_seq(__gnu_cxx::__mutex& lhs_mutex,
+ __gnu_debug::_Safe_sequence_base& lhs,
+ __gnu_cxx::__mutex& rhs_mutex,
+ __gnu_debug::_Safe_sequence_base& rhs)
{
- swap_seq(__lhs, __rhs);
+ lock_and_run(lhs_mutex, rhs_mutex,
+ [&lhs, &rhs]() { swap_seq_single(lhs, rhs); });
+ }
+
+ void
+ swap_ucont_single(__gnu_debug::_Safe_unordered_container_base& __lhs,
+ __gnu_debug::_Safe_unordered_container_base& __rhs)
+ {
+ swap_seq_single(__lhs, __rhs);
swap_its(__lhs, __lhs._M_local_iterators,
__rhs, __rhs._M_local_iterators);
swap_its(__lhs, __lhs._M_const_local_iterators,
@@ -92,6 +139,16 @@ namespace
}
void
+ swap_ucont(__gnu_cxx::__mutex& lhs_mutex,
+ __gnu_debug::_Safe_unordered_container_base& lhs,
+ __gnu_cxx::__mutex& rhs_mutex,
+ __gnu_debug::_Safe_unordered_container_base& rhs)
+ {
+ lock_and_run(lhs_mutex, rhs_mutex,
+ [&lhs, &rhs]() { swap_ucont_single(lhs, rhs); });
+ }
+
+ void
detach_all(__gnu_debug::_Safe_iterator_base* __iter)
{
for (; __iter;)
@@ -201,7 +258,8 @@ namespace __gnu_debug
_Safe_sequence_base::
_M_detach_all()
{
- __gnu_cxx::__scoped_lock sentry(_M_get_mutex());
+ // No need to lock as current instance is being destroyed.
+ // __gnu_cxx::__scoped_lock sentry(_M_get_mutex(alignment));
detach_all(_M_iterators);
_M_iterators = 0;
@@ -214,6 +272,21 @@ namespace __gnu_debug
_M_detach_singular()
{
__gnu_cxx::__scoped_lock sentry(_M_get_mutex());
+ _M_detach_singular_single();
+ }
+
+ void
+ _Safe_sequence_base::
+ _M_detach_singular(size_t alignment)
+ {
+ __gnu_cxx::__scoped_lock sentry(_M_get_mutex(alignment));
+ _M_detach_singular_single();
+ }
+
+ void
+ _Safe_sequence_base::
+ _M_detach_singular_single()
+ {
for (_Safe_iterator_base* __iter = _M_iterators; __iter;)
{
_Safe_iterator_base* __old = __iter;
@@ -236,6 +309,21 @@ namespace __gnu_debug
_M_revalidate_singular()
{
__gnu_cxx::__scoped_lock sentry(_M_get_mutex());
+ _M_revalidate_singular_single();
+ }
+
+ void
+ _Safe_sequence_base::
+ _M_revalidate_singular(std::size_t alignment)
+ {
+ __gnu_cxx::__scoped_lock sentry(_M_get_mutex(alignment));
+ _M_revalidate_singular_single();
+ }
+
+ void
+ _Safe_sequence_base::
+ _M_revalidate_singular_single()
+ {
for (_Safe_iterator_base* __iter = _M_iterators; __iter;
__iter = __iter->_M_next)
__iter->_M_version = _M_version;
@@ -249,23 +337,22 @@ namespace __gnu_debug
_Safe_sequence_base::
_M_swap(_Safe_sequence_base& __x) noexcept
{
- // We need to lock both sequences to swap
+ // We need to lock both containers to swap
using namespace __gnu_cxx;
- __mutex *__this_mutex = &_M_get_mutex();
- __mutex *__x_mutex = &__x._M_get_mutex();
- if (__this_mutex == __x_mutex)
- {
- __scoped_lock __lock(*__this_mutex);
- swap_seq(*this, __x);
- }
- else
- {
- __scoped_lock __l1(__this_mutex < __x_mutex
- ? *__this_mutex : *__x_mutex);
- __scoped_lock __l2(__this_mutex < __x_mutex
- ? *__x_mutex : *__this_mutex);
- swap_seq(*this, __x);
- }
+ __mutex& __this_mutex = _M_get_mutex();
+ __mutex& __x_mutex = __x._M_get_mutex();
+ swap_seq(__this_mutex, *this, __x_mutex, __x);
+ }
+
+ void
+ _Safe_sequence_base::
+ _M_swap(_Safe_sequence_base& __x, std::size_t alignment) noexcept
+ {
+ // We need to lock both containers to swap
+ using namespace __gnu_cxx;
+ __mutex& __this_mutex = _M_get_mutex(alignment);
+ __mutex& __x_mutex = __x._M_get_mutex(alignment);
+ swap_seq(__this_mutex, *this, __x_mutex, __x);
}
__gnu_cxx::__mutex&
@@ -273,6 +360,11 @@ namespace __gnu_debug
_M_get_mutex() throw ()
{ return get_safe_base_mutex(this); }
+ __gnu_cxx::__mutex&
+ _Safe_sequence_base::
+ _M_get_mutex(std::size_t alignment) throw ()
+ { return get_safe_base_mutex(this, alignment); }
+
void
_Safe_sequence_base::
_M_attach(_Safe_iterator_base* __it, bool __constant)
@@ -283,6 +375,14 @@ namespace __gnu_debug
void
_Safe_sequence_base::
+ _M_attach(_Safe_iterator_base* __it, bool __constant, std::size_t alignment)
+ {
+ __gnu_cxx::__scoped_lock sentry(_M_get_mutex(alignment));
+ _M_attach_single(__it, __constant);
+ }
+
+ void
+ _Safe_sequence_base::
_M_attach_single(_Safe_iterator_base* __it, bool __constant) throw ()
{
_Safe_iterator_base*& __its =
@@ -304,6 +404,15 @@ namespace __gnu_debug
void
_Safe_sequence_base::
+ _M_detach(_Safe_iterator_base* __it, std::size_t alignment)
+ {
+ // Remove __it from this sequence's list
+ __gnu_cxx::__scoped_lock sentry(_M_get_mutex(alignment));
+ _M_detach_single(__it);
+ }
+
+ void
+ _Safe_sequence_base::
_M_detach_single(_Safe_iterator_base* __it) throw ()
{
// Remove __it from this sequence's list
@@ -331,6 +440,21 @@ namespace __gnu_debug
void
_Safe_iterator_base::
+ _M_attach(_Safe_sequence_base* __seq, bool __constant, std::size_t alignment)
+ {
+ _M_detach(alignment);
+
+ // Attach to the new sequence (if there is one)
+ if (__seq)
+ {
+ _M_sequence = __seq;
+ _M_version = _M_sequence->_M_version;
+ _M_sequence->_M_attach(this, __constant, alignment);
+ }
+ }
+
+ void
+ _Safe_iterator_base::
_M_attach_single(_Safe_sequence_base* __seq, bool __constant) throw ()
{
_M_detach_single();
@@ -356,6 +480,16 @@ namespace __gnu_debug
void
_Safe_iterator_base::
+ _M_detach(std::size_t alignment)
+ {
+ if (_M_sequence)
+ _M_sequence->_M_detach(this, alignment);
+
+ _M_reset();
+ }
+
+ void
+ _Safe_iterator_base::
_M_detach_single() throw ()
{
if (_M_sequence)
@@ -389,8 +523,8 @@ namespace __gnu_debug
__gnu_cxx::__mutex&
_Safe_iterator_base::
- _M_get_mutex() throw ()
- { return get_safe_base_mutex(_M_sequence); }
+ _M_get_mutex() throw()
+ { return _M_sequence->_M_get_mutex(); }
_Safe_unordered_container_base*
_Safe_local_iterator_base::
@@ -414,6 +548,21 @@ namespace __gnu_debug
void
_Safe_local_iterator_base::
+ _M_attach(_Safe_sequence_base* __cont, bool __constant, std::size_t alignment)
+ {
+ _M_detach(alignment);
+
+ // Attach to the new container (if there is one)
+ if (__cont)
+ {
+ _M_sequence = __cont;
+ _M_version = _M_sequence->_M_version;
+ _M_get_container()->_M_attach_local(this, __constant, alignment);
+ }
+ }
+
+ void
+ _Safe_local_iterator_base::
_M_attach_single(_Safe_sequence_base* __cont, bool __constant) throw ()
{
_M_detach_single();
@@ -439,6 +588,16 @@ namespace __gnu_debug
void
_Safe_local_iterator_base::
+ _M_detach(std::size_t alignment)
+ {
+ if (_M_sequence)
+ _M_get_container()->_M_detach_local(this, alignment);
+
+ _M_reset();
+ }
+
+ void
+ _Safe_local_iterator_base::
_M_detach_single() throw ()
{
if (_M_sequence)
@@ -451,7 +610,8 @@ namespace __gnu_debug
_Safe_unordered_container_base::
_M_detach_all()
{
- __gnu_cxx::__scoped_lock sentry(_M_get_mutex());
+ // No need to lock as current instance is being destroyed.
+ // __gnu_cxx::__scoped_lock sentry(_M_get_mutex(alignment));
detach_all(_M_iterators);
_M_iterators = 0;
@@ -471,21 +631,20 @@ namespace __gnu_debug
{
// We need to lock both containers to swap
using namespace __gnu_cxx;
- __mutex *__this_mutex = &_M_get_mutex();
- __mutex *__x_mutex = &__x._M_get_mutex();
- if (__this_mutex == __x_mutex)
- {
- __scoped_lock __lock(*__this_mutex);
- swap_ucont(*this, __x);
- }
- else
- {
- __scoped_lock __l1(__this_mutex < __x_mutex
- ? *__this_mutex : *__x_mutex);
- __scoped_lock __l2(__this_mutex < __x_mutex
- ? *__x_mutex : *__this_mutex);
- swap_ucont(*this, __x);
- }
+ __mutex& __this_mutex = _M_get_mutex();
+ __mutex& __x_mutex = __x._M_get_mutex();
+ swap_ucont(__this_mutex, *this, __x_mutex, __x);
+ }
+
+ void
+ _Safe_unordered_container_base::
+ _M_swap(_Safe_unordered_container_base& __x, std::size_t alignment) noexcept
+ {
+ // We need to lock both containers to swap
+ using namespace __gnu_cxx;
+ __mutex& __this_mutex = _M_get_mutex(alignment);
+ __mutex& __x_mutex = __x._M_get_mutex(alignment);
+ swap_ucont(__this_mutex, *this, __x_mutex, __x);
}
void
@@ -498,6 +657,15 @@ namespace __gnu_debug
void
_Safe_unordered_container_base::
+ _M_attach_local(_Safe_iterator_base* __it, bool __constant,
+ std::size_t alignment)
+ {
+ __gnu_cxx::__scoped_lock sentry(_M_get_mutex(alignment));
+ _M_attach_local_single(__it, __constant);
+ }
+
+ void
+ _Safe_unordered_container_base::
_M_attach_local_single(_Safe_iterator_base* __it, bool __constant) throw ()
{
_Safe_iterator_base*& __its =
@@ -519,6 +687,15 @@ namespace __gnu_debug
void
_Safe_unordered_container_base::
+ _M_detach_local(_Safe_iterator_base* __it, std::size_t alignment)
+ {
+ // Remove __it from this container's list
+ __gnu_cxx::__scoped_lock sentry(_M_get_mutex(alignment));
+ _M_detach_local_single(__it);
+ }
+
+ void
+ _Safe_unordered_container_base::
_M_detach_local_single(_Safe_iterator_base* __it) throw ()
{
// Remove __it from this container's list
diff --git a/libstdc++-v3/testsuite/23_containers/vector/debug/mutex_association.cc b/libstdc++-v3/testsuite/23_containers/vector/debug/mutex_association.cc
new file mode 100644
index 0000000..b043548
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/debug/mutex_association.cc
@@ -0,0 +1,40 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+//
+
+#include <debug/vector>
+
+#include <testsuite_hooks.h>
+
+
+class container : public __gnu_debug::_Safe_sequence<container>
+{
+public:
+ __gnu_cxx::__mutex&
+ get_mutex()
+ { return this->_M_get_mutex(); }
+};
+
+int
+main()
+{
+ container conts[16];
+
+ for (int i = 1; i != 16; ++i)
+ VERIFY( &conts[i - 1].get_mutex() != &conts[i].get_mutex() );
+}
+
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-13 20:37 debug container mutex association François Dumont
@ 2016-09-14 9:01 ` Jonathan Wakely
2016-09-14 20:53 ` François Dumont
0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Wakely @ 2016-09-14 9:01 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 13/09/16 22:37 +0200, François Dumont wrote:
>Hi
>
> When I proposed to change std::hash for pointers my plan was to
>use this approach for the debug containers. So here is the patch
>leveraging on this technique to avoid going through _Hash_impl to hash
>address and get mutex index from it. I think it is obious that new
>access is faster so I didn't wrote a performance test to demonstrate
>it.
Is this actually a bottleneck that needs to be made faster?
I know it's nice if Debug Mode isn't very slow, but you've already
made it much better, and performance is not the primary goal of Debug
Mode.
>diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
>index 9b5bb23..c9a253e 100644
>--- a/libstdc++-v3/config/abi/pre/gnu.ver
>+++ b/libstdc++-v3/config/abi/pre/gnu.ver
>@@ -1947,6 +1947,21 @@ GLIBCXX_3.4.23 {
> _ZNSsC[12]ERKSs[jmy]RKSaIcE;
> _ZNSbIwSt11char_traitsIwESaIwEEC[12]ERKS2_mRKS1_;
>
>+ # __gnu_debug::_Safe_sequence_base
>+ _ZN11__gnu_debug19_Safe_sequence_base18_M_detach_singularEm;
The 'm' here should be [jmy] because std::size_t mangles differently
on different targets.
>+ _ZN11__gnu_debug19_Safe_sequence_base22_M_revalidate_singularEm;
>+ _ZN11__gnu_debug19_Safe_sequence_base12_M_get_mutexEm;
>+ _ZN11__gnu_debug19_Safe_sequence_base7_M_swapERS0_m;
>+
>+ # __gnu_debug::_Safe_iterator_base
>+ _ZN11__gnu_debug19_Safe_iterator_base9_M_attachEPNS_19_Safe_sequence_baseEbm;
>+ _ZN11__gnu_debug19_Safe_iterator_base9_M_detachEm;
>+
>+ # __gnu_debug::_Safe_unordered_container_base and _Safe_local_iterator_base
>+ _ZN11__gnu_debug30_Safe_unordered_container_base7_M_swapERS0_m;
>+ _ZN11__gnu_debug25_Safe_local_iterator_base9_M_attachEPNS_19_Safe_sequence_baseEbm;
>+ _ZN11__gnu_debug25_Safe_local_iterator_base9_M_detachEm;
>+
> } GLIBCXX_3.4.22;
It's unfortunate to have to export and maintain new symbols when we
don't offer the same level of ABI guarantee for Debug Mode :-(
>@@ -174,6 +191,18 @@ namespace __gnu_debug
> const _Safe_iterator_base* __last)
> { return __first->_M_can_compare(*__last); }
>
>+ // Compute power of 2 not greater than __n.
>+ template<size_t __n>
>+ struct _Lowest_power_of_two
>+ {
>+ static const size_t __val
>+ = _Lowest_power_of_two< (__n >> 1) >::__val + 1;
>+ };
>+
>+ template<>
>+ struct _Lowest_power_of_two<1>
>+ { static const size_t __val = 1; };
The lowest power of two not greater than 1 is 2^0, but this gives the
result 1.
Similarly, the lowest power of two not greater than 2 is 2^1, i.e. 1.
And the name is misleading, it doesn't compute a power of two, it
computes the base-2 logarithm (then adds one), so it's a compile-time
version of floor(log2(n))+1.
>@@ -123,6 +125,36 @@ namespace __gnu_debug
> template<typename _Predicate>
> void
> _M_transfer_from_if(_Safe_sequence& __from, _Predicate __pred);
>+
>+ static _GLIBCXX_CONSTEXPR std::size_t
>+ _S_alignment() _GLIBCXX_NOEXCEPT
>+ { return _Lowest_power_of_two<__alignof(_Sequence)>::__val; }
This function is called "alignment" but it returns log2(alignof(T))+1
Does it need to be constexpr? Is that for std::array?
You can get the same result without _Lowest_power_of_two:
static _GLIBCXX_CONSTEXPR size_t
_S_alignbits() _GLIBCXX_NOEXCEPT
{ return __builtin_ctz(__alignof(_Sequence)) + 1; }
But I'm not convinced the +1 is correct, see below.
>@@ -46,15 +47,30 @@ namespace
> /** Returns different instances of __mutex depending on the passed address
> * in order to limit contention without breaking current library binary
> * compatibility. */
>+ const size_t mask = 0xf;
>+
> __gnu_cxx::__mutex&
>- get_safe_base_mutex(void* __address)
>+ get_mutex(size_t index)
> {
>- const size_t mask = 0xf;
> static __gnu_cxx::__mutex safe_base_mutex[mask + 1];
>- const size_t index = _Hash_impl::hash(__address) & mask;
> return safe_base_mutex[index];
> }
N.B. https://gcc.gnu.org/PR71312
>
>+ __gnu_cxx::__mutex&
>+ get_safe_base_mutex(void* address)
>+ {
>+ const size_t index = _Hash_impl::hash(address) & mask;
>+ return get_mutex(index);
>+ }
>+
>+ __gnu_cxx::__mutex&
>+ get_safe_base_mutex(void* address, std::size_t alignment)
>+ {
>+ const size_t index
>+ = (reinterpret_cast<size_t>(address) >> alignment) & mask;
>+ return get_mutex(index);
>+ }
Since the "alignment" value (which is really log2(align)+1) is always
a positive value that means we always shift the address. But that
means we drop the least significant bit of all pointers, even when the
type has alignof(T)==1 and so all bits in the pointer are significant.
Is that intentional? Is the idea to only drop bits that are always
zero?
By changing the algorithm we use to map an address to a mutex do we
allow programs to get two different mutexes for the same object? That
can only happen when some objects are compiled with an older GCC and
so still use the old _M_get_mutex() function that doesn't take an
argument. We require all objects to be rebuilt with the same compiler
for Debug Mode (but maybe not if using __gnu_debug::vector etc.
explicitly in normal mode?) so that's not a big deal. But it's a pity
we have to keep the old symbols around if using them is unsafe.
> void
> swap_its(__gnu_debug::_Safe_sequence_base& __lhs,
> __gnu_debug::_Safe_iterator_base*& __lhs_its,
>@@ -70,8 +86,8 @@ namespace
> }
>
> void
>- swap_seq(__gnu_debug::_Safe_sequence_base& __lhs,
>- __gnu_debug::_Safe_sequence_base& __rhs)
>+ swap_seq_single(__gnu_debug::_Safe_sequence_base& __lhs,
>+ __gnu_debug::_Safe_sequence_base& __rhs)
> {
> swap(__lhs._M_version, __rhs._M_version);
> swap_its(__lhs, __lhs._M_iterators,
>@@ -81,10 +97,41 @@ namespace
> }
>
> void
>- swap_ucont(__gnu_debug::_Safe_unordered_container_base& __lhs,
>- __gnu_debug::_Safe_unordered_container_base& __rhs)
>+ lock_and_run(__gnu_cxx::__mutex& lhs_mutex,
>+ __gnu_cxx::__mutex& rhs_mutex,
>+ std::function<void()> action)
Do all calls to this fit into the std::function small-object buffer,
so we don't need to allocate memory for it?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-14 9:01 ` Jonathan Wakely
@ 2016-09-14 20:53 ` François Dumont
2016-09-15 9:15 ` Jonathan Wakely
0 siblings, 1 reply; 16+ messages in thread
From: François Dumont @ 2016-09-14 20:53 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
On 14/09/2016 11:00, Jonathan Wakely wrote:
> On 13/09/16 22:37 +0200, François Dumont wrote:
>> Hi
>>
>> When I proposed to change std::hash for pointers my plan was to
>> use this approach for the debug containers. So here is the patch
>> leveraging on this technique to avoid going through _Hash_impl to
>> hash address and get mutex index from it. I think it is obious that
>> new access is faster so I didn't wrote a performance test to
>> demonstrate it.
>
> Is this actually a bottleneck that needs to be made faster?
>
> I know it's nice if Debug Mode isn't very slow, but you've already
> made it much better, and performance is not the primary goal of Debug
> Mode.
Sure but my approach is that if I can make something faster then I just
try. I considered that making this mode faster will allow its usage in
an even more extended way.
>
>> diff --git a/libstdc++-v3/config/abi/pre/gnu.ver
>> b/libstdc++-v3/config/abi/pre/gnu.ver
>> index 9b5bb23..c9a253e 100644
>> --- a/libstdc++-v3/config/abi/pre/gnu.ver
>> +++ b/libstdc++-v3/config/abi/pre/gnu.ver
>> @@ -1947,6 +1947,21 @@ GLIBCXX_3.4.23 {
>> _ZNSsC[12]ERKSs[jmy]RKSaIcE;
>> _ZNSbIwSt11char_traitsIwESaIwEEC[12]ERKS2_mRKS1_;
>>
>> + # __gnu_debug::_Safe_sequence_base
>> + _ZN11__gnu_debug19_Safe_sequence_base18_M_detach_singularEm;
>
> The 'm' here should be [jmy] because std::size_t mangles differently
> on different targets.
>
>> + _ZN11__gnu_debug19_Safe_sequence_base22_M_revalidate_singularEm;
>> + _ZN11__gnu_debug19_Safe_sequence_base12_M_get_mutexEm;
>> + _ZN11__gnu_debug19_Safe_sequence_base7_M_swapERS0_m;
>> +
>> + # __gnu_debug::_Safe_iterator_base
>> +
>> _ZN11__gnu_debug19_Safe_iterator_base9_M_attachEPNS_19_Safe_sequence_baseEbm;
>> + _ZN11__gnu_debug19_Safe_iterator_base9_M_detachEm;
>> +
>> + # __gnu_debug::_Safe_unordered_container_base and
>> _Safe_local_iterator_base
>> + _ZN11__gnu_debug30_Safe_unordered_container_base7_M_swapERS0_m;
>> +
>> _ZN11__gnu_debug25_Safe_local_iterator_base9_M_attachEPNS_19_Safe_sequence_baseEbm;
>> + _ZN11__gnu_debug25_Safe_local_iterator_base9_M_detachEm;
>> +
>> } GLIBCXX_3.4.22;
>
> It's unfortunate to have to export and maintain new symbols when we
> don't offer the same level of ABI guarantee for Debug Mode :-(
Yes, I have been told that Debug ABI compatibility was not guaranteed
but at the same time if we don't do so abi-check is failing. Couldn't we
have a special section in gnu.ver for unversioned symbols like those ?
>
>> @@ -174,6 +191,18 @@ namespace __gnu_debug
>> const _Safe_iterator_base* __last)
>> { return __first->_M_can_compare(*__last); }
>>
>> + // Compute power of 2 not greater than __n.
>> + template<size_t __n>
>> + struct _Lowest_power_of_two
>> + {
>> + static const size_t __val
>> + = _Lowest_power_of_two< (__n >> 1) >::__val + 1;
>> + };
>> +
>> + template<>
>> + struct _Lowest_power_of_two<1>
>> + { static const size_t __val = 1; };
>
> The lowest power of two not greater than 1 is 2^0, but this gives the
> result 1.
>
> Similarly, the lowest power of two not greater than 2 is 2^1, i.e. 1.
Indeed, when writing the test case I decided to return 1 rather than 0
for the _Lowest_power_of_two<1> specialization.It gave a better
container instance - mutex mapping. But I forgot to change its name.
>
> And the name is misleading, it doesn't compute a power of two, it
> computes the base-2 logarithm (then adds one), so it's a compile-time
> version of floor(log2(n))+1.
>
>> @@ -123,6 +125,36 @@ namespace __gnu_debug
>> template<typename _Predicate>
>> void
>> _M_transfer_from_if(_Safe_sequence& __from, _Predicate __pred);
>> +
>> + static _GLIBCXX_CONSTEXPR std::size_t
>> + _S_alignment() _GLIBCXX_NOEXCEPT
>> + { return _Lowest_power_of_two<__alignof(_Sequence)>::__val; }
>
> This function is called "alignment" but it returns log2(alignof(T))+1
Yes, I wasn't proud about the naming.
>
> Does it need to be constexpr? Is that for std::array?
No, it is not used in std::array context. I just considered that it
could be constexpr.
>
> You can get the same result without _Lowest_power_of_two:
>
> static _GLIBCXX_CONSTEXPR size_t
> _S_alignbits() _GLIBCXX_NOEXCEPT
> { return __builtin_ctz(__alignof(_Sequence)) + 1; }
>
> But I'm not convinced the +1 is correct, see below.
>
>> @@ -46,15 +47,30 @@ namespace
>> /** Returns different instances of __mutex depending on the passed
>> address
>> * in order to limit contention without breaking current library
>> binary
>> * compatibility. */
>> + const size_t mask = 0xf;
>> +
>> __gnu_cxx::__mutex&
>> - get_safe_base_mutex(void* __address)
>> + get_mutex(size_t index)
>> {
>> - const size_t mask = 0xf;
>> static __gnu_cxx::__mutex safe_base_mutex[mask + 1];
>> - const size_t index = _Hash_impl::hash(__address) & mask;
>> return safe_base_mutex[index];
>> }
>
> N.B. https://gcc.gnu.org/PR71312
Ok, debug mode is also impacted. Shouldn't the alignment be set on the
__mutex type directly ?
>
>
>>
>> + __gnu_cxx::__mutex&
>> + get_safe_base_mutex(void* address)
>> + {
>> + const size_t index = _Hash_impl::hash(address) & mask;
>> + return get_mutex(index);
>> + }
>> +
>> + __gnu_cxx::__mutex&
>> + get_safe_base_mutex(void* address, std::size_t alignment)
>> + {
>> + const size_t index
>> + = (reinterpret_cast<size_t>(address) >> alignment) & mask;
>> + return get_mutex(index);
>> + }
>
> Since the "alignment" value (which is really log2(align)+1) is always
> a positive value that means we always shift the address. But that
> means we drop the least significant bit of all pointers, even when the
> type has alignof(T)==1 and so all bits in the pointer are significant.
> Is that intentional? Is the idea to only drop bits that are always
> zero?
Yes, to make sure we get use of all available mutexes.
>
> By changing the algorithm we use to map an address to a mutex do we
> allow programs to get two different mutexes for the same object? That
> can only happen when some objects are compiled with an older GCC and
> so still use the old _M_get_mutex() function that doesn't take an
> argument. We require all objects to be rebuilt with the same compiler
> for Debug Mode (but maybe not if using __gnu_debug::vector etc.
> explicitly in normal mode?) so that's not a big deal. But it's a pity
> we have to keep the old symbols around if using them is unsafe.
Yes, it could happen even if very unlikely. I would also prefer to get
rid of former symbols but can we ?
>
>> void
>> swap_its(__gnu_debug::_Safe_sequence_base& __lhs,
>> __gnu_debug::_Safe_iterator_base*& __lhs_its,
>> @@ -70,8 +86,8 @@ namespace
>> }
>>
>> void
>> - swap_seq(__gnu_debug::_Safe_sequence_base& __lhs,
>> - __gnu_debug::_Safe_sequence_base& __rhs)
>> + swap_seq_single(__gnu_debug::_Safe_sequence_base& __lhs,
>> + __gnu_debug::_Safe_sequence_base& __rhs)
>> {
>> swap(__lhs._M_version, __rhs._M_version);
>> swap_its(__lhs, __lhs._M_iterators,
>> @@ -81,10 +97,41 @@ namespace
>> }
>>
>> void
>> - swap_ucont(__gnu_debug::_Safe_unordered_container_base& __lhs,
>> - __gnu_debug::_Safe_unordered_container_base& __rhs)
>> + lock_and_run(__gnu_cxx::__mutex& lhs_mutex,
>> + __gnu_cxx::__mutex& rhs_mutex,
>> + std::function<void()> action)
>
> Do all calls to this fit into the std::function small-object buffer,
> so we don't need to allocate memory for it?
>
I didn't consider this. I thought it was a nice way to avoid making
lock_and_run a template function to take the lambda.
Reading this I realize that we could perhaps acheive the same without
touching as much code. __alignof of container types will always give the
same value on a give target no ? Does it depends on the type used to
instantiate the container ? Isn't there some macro giving the target
alignment that I could use ?
François
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-14 20:53 ` François Dumont
@ 2016-09-15 9:15 ` Jonathan Wakely
2016-09-19 20:19 ` François Dumont
0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Wakely @ 2016-09-15 9:15 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 14/09/16 22:17 +0200, François Dumont wrote:
>On 14/09/2016 11:00, Jonathan Wakely wrote:
>>On 13/09/16 22:37 +0200, François Dumont wrote:
>>>Hi
>>>
>>> When I proposed to change std::hash for pointers my plan was to
>>>use this approach for the debug containers. So here is the patch
>>>leveraging on this technique to avoid going through _Hash_impl to
>>>hash address and get mutex index from it. I think it is obious
>>>that new access is faster so I didn't wrote a performance test to
>>>demonstrate it.
>>
>>Is this actually a bottleneck that needs to be made faster?
>>
>>I know it's nice if Debug Mode isn't very slow, but you've already
>>made it much better, and performance is not the primary goal of Debug
>>Mode.
>
>Sure but my approach is that if I can make something faster then I
>just try. I considered that making this mode faster will allow its
>usage in an even more extended way.
>
>>
>>>diff --git a/libstdc++-v3/config/abi/pre/gnu.ver
>>>b/libstdc++-v3/config/abi/pre/gnu.ver
>>>index 9b5bb23..c9a253e 100644
>>>--- a/libstdc++-v3/config/abi/pre/gnu.ver
>>>+++ b/libstdc++-v3/config/abi/pre/gnu.ver
>>>@@ -1947,6 +1947,21 @@ GLIBCXX_3.4.23 {
>>> _ZNSsC[12]ERKSs[jmy]RKSaIcE;
>>> _ZNSbIwSt11char_traitsIwESaIwEEC[12]ERKS2_mRKS1_;
>>>
>>>+ # __gnu_debug::_Safe_sequence_base
>>>+ _ZN11__gnu_debug19_Safe_sequence_base18_M_detach_singularEm;
>>
>>The 'm' here should be [jmy] because std::size_t mangles differently
>>on different targets.
>>
>>>+ _ZN11__gnu_debug19_Safe_sequence_base22_M_revalidate_singularEm;
>>>+ _ZN11__gnu_debug19_Safe_sequence_base12_M_get_mutexEm;
>>>+ _ZN11__gnu_debug19_Safe_sequence_base7_M_swapERS0_m;
>>>+
>>>+ # __gnu_debug::_Safe_iterator_base
>>>+ _ZN11__gnu_debug19_Safe_iterator_base9_M_attachEPNS_19_Safe_sequence_baseEbm;
>>>+ _ZN11__gnu_debug19_Safe_iterator_base9_M_detachEm;
>>>+
>>>+ # __gnu_debug::_Safe_unordered_container_base and
>>>_Safe_local_iterator_base
>>>+ _ZN11__gnu_debug30_Safe_unordered_container_base7_M_swapERS0_m;
>>>+ _ZN11__gnu_debug25_Safe_local_iterator_base9_M_attachEPNS_19_Safe_sequence_baseEbm;
>>>+ _ZN11__gnu_debug25_Safe_local_iterator_base9_M_detachEm;
>>>+
>>>} GLIBCXX_3.4.22;
>>
>>It's unfortunate to have to export and maintain new symbols when we
>>don't offer the same level of ABI guarantee for Debug Mode :-(
>
>Yes, I have been told that Debug ABI compatibility was not guaranteed
>but at the same time if we don't do so abi-check is failing. Couldn't
>we have a special section in gnu.ver for unversioned symbols like
>those ?
I we could, but making them unversioned wouldn't really be useful - we
still need to keep them in the library forever.
>>>@@ -174,6 +191,18 @@ namespace __gnu_debug
>>> const _Safe_iterator_base* __last)
>>> { return __first->_M_can_compare(*__last); }
>>>
>>>+ // Compute power of 2 not greater than __n.
>>>+ template<size_t __n>
>>>+ struct _Lowest_power_of_two
>>>+ {
>>>+ static const size_t __val
>>>+ = _Lowest_power_of_two< (__n >> 1) >::__val + 1;
>>>+ };
>>>+
>>>+ template<>
>>>+ struct _Lowest_power_of_two<1>
>>>+ { static const size_t __val = 1; };
>>
>>The lowest power of two not greater than 1 is 2^0, but this gives the
>>result 1.
>>
>>Similarly, the lowest power of two not greater than 2 is 2^1, i.e. 1.
>
>Indeed, when writing the test case I decided to return 1 rather than 0
>for the _Lowest_power_of_two<1> specialization.It gave a better
>container instance - mutex mapping. But I forgot to change its name.
>
>>
>>And the name is misleading, it doesn't compute a power of two, it
>>computes the base-2 logarithm (then adds one), so it's a compile-time
>>version of floor(log2(n))+1.
>>
>>>@@ -123,6 +125,36 @@ namespace __gnu_debug
>>> template<typename _Predicate>
>>> void
>>> _M_transfer_from_if(_Safe_sequence& __from, _Predicate __pred);
>>>+
>>>+ static _GLIBCXX_CONSTEXPR std::size_t
>>>+ _S_alignment() _GLIBCXX_NOEXCEPT
>>>+ { return _Lowest_power_of_two<__alignof(_Sequence)>::__val; }
>>
>>This function is called "alignment" but it returns log2(alignof(T))+1
>
>Yes, I wasn't proud about the naming.
>
>>
>>Does it need to be constexpr? Is that for std::array?
>
>No, it is not used in std::array context. I just considered that it
>could be constexpr.
OK. Sometimes that's useful, I agree. But it's never used in a
constexpr context, and it's only called by the library directly, so
there's no advantage here (it just makes it harder to write because it
can only use code that's valid in a constant expression).
Anyway, you can write it without a template, and still be constexpr:
>>
>>You can get the same result without _Lowest_power_of_two:
>>
>> static _GLIBCXX_CONSTEXPR size_t
>> _S_alignbits() _GLIBCXX_NOEXCEPT
>> { return __builtin_ctz(__alignof(_Sequence)) + 1; }
>>
>>But I'm not convinced the +1 is correct, see below.
>>
>>>@@ -46,15 +47,30 @@ namespace
>>> /** Returns different instances of __mutex depending on the
>>>passed address
>>> * in order to limit contention without breaking current
>>>library binary
>>> * compatibility. */
>>>+ const size_t mask = 0xf;
>>>+
>>> __gnu_cxx::__mutex&
>>>- get_safe_base_mutex(void* __address)
>>>+ get_mutex(size_t index)
>>> {
>>>- const size_t mask = 0xf;
>>> static __gnu_cxx::__mutex safe_base_mutex[mask + 1];
>>>- const size_t index = _Hash_impl::hash(__address) & mask;
>>> return safe_base_mutex[index];
>>> }
>>
>>N.B. https://gcc.gnu.org/PR71312
>Ok, debug mode is also impacted. Shouldn't the alignment be set on the
>__mutex type directly ?
No, definitely not. Apart from the fact that it would be an ABI
change, it would mean that struct X { __mutex mx; int i; } would not
place the int right next to the mutex, it would force it onto a
different cacheline. We want our arrays of mutexes to be on separate
cachelines, but most uses of __mutex are not in an array.
We probably can't fix this properly yet, because we don't have the
hardware_destructive_interference_size value. We could just make a
conservative estimate of 64 bytes though.
>>
>>
>>>
>>>+ __gnu_cxx::__mutex&
>>>+ get_safe_base_mutex(void* address)
>>>+ {
>>>+ const size_t index = _Hash_impl::hash(address) & mask;
>>>+ return get_mutex(index);
>>>+ }
>>>+
>>>+ __gnu_cxx::__mutex&
>>>+ get_safe_base_mutex(void* address, std::size_t alignment)
>>>+ {
>>>+ const size_t index
>>>+ = (reinterpret_cast<size_t>(address) >> alignment) & mask;
>>>+ return get_mutex(index);
>>>+ }
>>
>>Since the "alignment" value (which is really log2(align)+1) is always
>>a positive value that means we always shift the address. But that
>>means we drop the least significant bit of all pointers, even when the
>>type has alignof(T)==1 and so all bits in the pointer are significant.
>>Is that intentional? Is the idea to only drop bits that are always
>>zero?
>
>Yes, to make sure we get use of all available mutexes.
OK, so the +1 is not needed?
>>
>>By changing the algorithm we use to map an address to a mutex do we
>>allow programs to get two different mutexes for the same object? That
>>can only happen when some objects are compiled with an older GCC and
>>so still use the old _M_get_mutex() function that doesn't take an
>>argument. We require all objects to be rebuilt with the same compiler
>>for Debug Mode (but maybe not if using __gnu_debug::vector etc.
>>explicitly in normal mode?) so that's not a big deal. But it's a pity
>>we have to keep the old symbols around if using them is unsafe.
>
>Yes, it could happen even if very unlikely. I would also prefer to get
>rid of former symbols but can we ?
No. Objects built against an older libstdc++ would fail to run when
the dynamic linker finds a new libstdc++.so.
We can't remove exported symbols from the DSO.
>>
>>> void
>>> swap_its(__gnu_debug::_Safe_sequence_base& __lhs,
>>> __gnu_debug::_Safe_iterator_base*& __lhs_its,
>>>@@ -70,8 +86,8 @@ namespace
>>> }
>>>
>>> void
>>>- swap_seq(__gnu_debug::_Safe_sequence_base& __lhs,
>>>- __gnu_debug::_Safe_sequence_base& __rhs)
>>>+ swap_seq_single(__gnu_debug::_Safe_sequence_base& __lhs,
>>>+ __gnu_debug::_Safe_sequence_base& __rhs)
>>> {
>>> swap(__lhs._M_version, __rhs._M_version);
>>> swap_its(__lhs, __lhs._M_iterators,
>>>@@ -81,10 +97,41 @@ namespace
>>> }
>>>
>>> void
>>>- swap_ucont(__gnu_debug::_Safe_unordered_container_base& __lhs,
>>>- __gnu_debug::_Safe_unordered_container_base& __rhs)
>>>+ lock_and_run(__gnu_cxx::__mutex& lhs_mutex,
>>>+ __gnu_cxx::__mutex& rhs_mutex,
>>>+ std::function<void()> action)
>>
>>Do all calls to this fit into the std::function small-object buffer,
>>so we don't need to allocate memory for it?
>>
>I didn't consider this. I thought it was a nice way to avoid making
>lock_and_run a template function to take the lambda.
>
>Reading this I realize that we could perhaps acheive the same without
>touching as much code. __alignof of container types will always give
>the same value on a give target no ? Does it depends on the type used
>to instantiate the container ? Isn't there some macro giving the
>target alignment that I could use ?
Alignment isn't a single value for a target, e.g. for some targets
alignof(void*) != alignof(int), so which "target alignment" do you
want?
It seems likely that all the debug containers will be aligned to at
least alignof(void*), because of the _Safe_base::_M_sequence pointer
member. It should be OK to assume alignof(*this) == alignof(void*).
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-15 9:15 ` Jonathan Wakely
@ 2016-09-19 20:19 ` François Dumont
2016-09-20 8:58 ` Jonathan Wakely
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: François Dumont @ 2016-09-19 20:19 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 3612 bytes --]
Hi
Following our conversation here is a much simpler patch. I just
consider that all debug containers will have the same alignment.
Even if I submit this patch as a whole I will commit into pieces,
at least one for the pure cleanup parts and one for the debug.cc change.
Among those changes there is:
- __gnu_cxx::__scoped_lock(this->_M_get_mutex());
+ __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
I would appreciate if you could tell me what was happening with the
initial expression. I don't understand why it is compiling. I even tried
the same in debug.cc and started having compilation errors.
* include/debug/bitset (bitset::reference::reference(const _Base_ref&,
bitset*)): Remove __unused__ attribute.
* include/debug/safe_base.h (_Safe_iterator_base): Make
_Safe_sequence_base a friend.
(_Safe_iterator_base::_M_attach): Make protected.
(_Safe_iterator_base::_M_attach_single): Likewise.
(_Safe_iterator_base::_M_detach): Likewise.
(_Safe_iterator_base::_M_detach_single): Likewise.
(_Safe_sequence_base): Make _Safe_iterator_base a friend.
(_Safe_sequence_base::_Safe_sequence_base(_Safe_sequence_base&&)): New.
(_Safe_sequence_base::_M_swap): Make protected.
(_Safe_sequence_base::_M_attach): Make private.
(_Safe_sequence_base::_M_attach_single): Likewise.
(_Safe_sequence_base::_M_detach): Likewise.
(_Safe_sequence_base::_M_detach_single): Likewise.
* include/debug/safe_container.h
(_Safe_container::_Safe_container(_Safe_container&&)): Make default.
* include/debug/safe_iterator.h
(_Safe_iterator::operator++()): Name __scoped_lock instance.
* include/debug/safe_iterator.tcc: Remove trailing line.
* include/debug/safe_unordered_base.h
(_Safe_local_iterator_base::_M_attach): Make protected.
(_Safe_local_iterator_base::_M_attach_single): Likewise.
(_Safe_local_iterator_base::_M_detach): Likewise.
(_Safe_local_iterator_base::_M_detach_single): Likewise.
(_Safe_unordered_container_base): Make _Safe_local_iterator_base
friend.
(_Safe_unordered_container_base::_M_attach_local): Make private.
(_Safe_unordered_container_base::_M_attach_local_single): Likewise.
(_Safe_unordered_container_base::_M_detach_local): Likewise.
(_Safe_unordered_container_base::_M_detach_local_single): Likewise.
* src/c++11/debug.cc: Include debug/vector. Include cctype. Remove
functional.
(get_safe_base_mutex): Get mutex based on address lowest non nil bits.
* testsuite/23_containers/vector/debug/mutex_association.cc: New.
Tested under Linux x86_64.
Ok to commit ?
On 15/09/2016 10:51, Jonathan Wakely wrote:
> N.B. https://gcc.gnu.org/PR71312
>> Ok, debug mode is also impacted. Shouldn't the alignment be set on
>> the __mutex type directly ?
>
> No, definitely not. Apart from the fact that it would be an ABI
> change, it would mean that struct X { __mutex mx; int i; } would not
> place the int right next to the mutex, it would force it onto a
> different cacheline. We want our arrays of mutexes to be on separate
> cachelines, but most uses of __mutex are not in an array.
>
> We probably can't fix this properly yet, because we don't have the
> hardware_destructive_interference_size value. We could just make a
> conservative estimate of 64 bytes though.
Thanks for explaining that it is to avoid false sharing.
Maybe we could share this mutex pool with debug mode. This way the day
we fix this false sharing issue it will benefit to both shared_ptr and
debug mode.
François
[-- Attachment #2: debug_mutex.patch --]
[-- Type: text/x-patch, Size: 14582 bytes --]
diff --git a/libstdc++-v3/include/debug/bitset b/libstdc++-v3/include/debug/bitset
index 55d3281..b7bada3 100644
--- a/libstdc++-v3/include/debug/bitset
+++ b/libstdc++-v3/include/debug/bitset
@@ -66,8 +66,7 @@ namespace __debug
friend class bitset;
reference();
- reference(const _Base_ref& __base,
- bitset* __seq __attribute__((__unused__))) _GLIBCXX_NOEXCEPT
+ reference(const _Base_ref& __base, bitset* __seq) _GLIBCXX_NOEXCEPT
: _Base_ref(__base)
, _Safe_iterator_base(__seq, false)
{ }
@@ -81,7 +80,7 @@ namespace __debug
reference&
operator=(bool __x) _GLIBCXX_NOEXCEPT
{
- _GLIBCXX_DEBUG_VERIFY(! this->_M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_write)
._M_iterator(*this));
*static_cast<_Base_ref*>(this) = __x;
@@ -91,10 +90,10 @@ namespace __debug
reference&
operator=(const reference& __x) _GLIBCXX_NOEXCEPT
{
- _GLIBCXX_DEBUG_VERIFY(! __x._M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!__x._M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_read)
._M_iterator(__x));
- _GLIBCXX_DEBUG_VERIFY(! this->_M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_write)
._M_iterator(*this));
*static_cast<_Base_ref*>(this) = __x;
@@ -104,7 +103,7 @@ namespace __debug
bool
operator~() const _GLIBCXX_NOEXCEPT
{
- _GLIBCXX_DEBUG_VERIFY(! this->_M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_read)
._M_iterator(*this));
return ~(*static_cast<const _Base_ref*>(this));
@@ -112,7 +111,7 @@ namespace __debug
operator bool() const _GLIBCXX_NOEXCEPT
{
- _GLIBCXX_DEBUG_VERIFY(! this->_M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_read)
._M_iterator(*this));
return *static_cast<const _Base_ref*>(this);
@@ -121,7 +120,7 @@ namespace __debug
reference&
flip() _GLIBCXX_NOEXCEPT
{
- _GLIBCXX_DEBUG_VERIFY(! this->_M_singular(),
+ _GLIBCXX_DEBUG_VERIFY(!this->_M_singular(),
_M_message(__gnu_debug::__msg_bad_bitset_flip)
._M_iterator(*this));
_Base_ref::flip();
diff --git a/libstdc++-v3/include/debug/safe_base.h b/libstdc++-v3/include/debug/safe_base.h
index aad9c52..abdee73 100644
--- a/libstdc++-v3/include/debug/safe_base.h
+++ b/libstdc++-v3/include/debug/safe_base.h
@@ -49,6 +49,8 @@ namespace __gnu_debug
*/
class _Safe_iterator_base
{
+ friend class _Safe_sequence_base;
+
public:
/** The sequence this iterator references; may be NULL to indicate
a singular iterator. */
@@ -101,7 +103,6 @@ namespace __gnu_debug
__gnu_cxx::__mutex&
_M_get_mutex() throw ();
- public:
/** Attaches this iterator to the given sequence, detaching it
* from whatever sequence it was attached to originally. If the
* new sequence is the NULL pointer, the iterator is left
@@ -124,6 +125,7 @@ namespace __gnu_debug
void
_M_detach_single() throw ();
+ public:
/** Determines if we are attached to the given sequence. */
bool
_M_attached_to(const _Safe_sequence_base* __seq) const
@@ -193,6 +195,8 @@ namespace __gnu_debug
*/
class _Safe_sequence_base
{
+ friend class _Safe_iterator_base;
+
public:
/// The list of mutable iterators that reference this container
_Safe_iterator_base* _M_iterators;
@@ -212,6 +216,11 @@ namespace __gnu_debug
#if __cplusplus >= 201103L
_Safe_sequence_base(const _Safe_sequence_base&) noexcept
: _Safe_sequence_base() { }
+
+ // Move constructor swap iterators.
+ _Safe_sequence_base(_Safe_sequence_base&& __seq) noexcept
+ : _Safe_sequence_base()
+ { _M_swap(__seq); }
#endif
/** Notify all iterators that reference this sequence that the
@@ -250,12 +259,12 @@ namespace __gnu_debug
__gnu_cxx::__mutex&
_M_get_mutex() throw ();
- public:
/** Invalidates all iterators. */
void
_M_invalidate_all() const
{ if (++_M_version == 0) _M_version = 1; }
+ private:
/** Attach an iterator to this sequence. */
void
_M_attach(_Safe_iterator_base* __it, bool __constant);
diff --git a/libstdc++-v3/include/debug/safe_container.h b/libstdc++-v3/include/debug/safe_container.h
index b73f68c..d96cb17 100644
--- a/libstdc++-v3/include/debug/safe_container.h
+++ b/libstdc++-v3/include/debug/safe_container.h
@@ -55,12 +55,9 @@ namespace __gnu_debug
#if __cplusplus >= 201103L
_Safe_container() = default;
_Safe_container(const _Safe_container&) = default;
- _Safe_container(_Safe_container&& __x) noexcept
- : _Safe_container()
- { _Base::_M_swap(__x); }
+ _Safe_container(_Safe_container&&) = default;
- _Safe_container(_Safe_container&& __x,
- const _Alloc& __a)
+ _Safe_container(_Safe_container&& __x, const _Alloc& __a)
: _Safe_container()
{
if (__x._M_cont().get_allocator() == __a)
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index c95d36c..41bc93a 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -295,7 +295,7 @@ namespace __gnu_debug
_GLIBCXX_DEBUG_VERIFY(this->_M_incrementable(),
_M_message(__msg_bad_inc)
._M_iterator(*this, "this"));
- __gnu_cxx::__scoped_lock(this->_M_get_mutex());
+ __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
++base();
return *this;
}
diff --git a/libstdc++-v3/include/debug/safe_iterator.tcc b/libstdc++-v3/include/debug/safe_iterator.tcc
index e8e1b00..0ae7fd1 100644
--- a/libstdc++-v3/include/debug/safe_iterator.tcc
+++ b/libstdc++-v3/include/debug/safe_iterator.tcc
@@ -93,4 +93,3 @@ namespace __gnu_debug
} // namespace __gnu_debug
#endif
-
diff --git a/libstdc++-v3/include/debug/safe_unordered_base.h b/libstdc++-v3/include/debug/safe_unordered_base.h
index 82a42eb..21292c3 100644
--- a/libstdc++-v3/include/debug/safe_unordered_base.h
+++ b/libstdc++-v3/include/debug/safe_unordered_base.h
@@ -76,24 +76,27 @@ namespace __gnu_debug
_Safe_unordered_container_base*
_M_get_container() const noexcept;
- public:
/** Attaches this iterator to the given container, detaching it
* from whatever container it was attached to originally. If the
* new container is the NULL pointer, the iterator is left
* unattached.
*/
- void _M_attach(_Safe_sequence_base* __seq, bool __constant);
+ void
+ _M_attach(_Safe_sequence_base* __seq, bool __constant);
/** Likewise, but not thread-safe. */
- void _M_attach_single(_Safe_sequence_base* __seq, bool __constant) throw ();
+ void
+ _M_attach_single(_Safe_sequence_base* __seq, bool __constant) throw ();
/** Detach the iterator for whatever container it is attached to,
* if any.
*/
- void _M_detach();
+ void
+ _M_detach();
/** Likewise, but not thread-safe. */
- void _M_detach_single() throw ();
+ void
+ _M_detach_single() throw ();
};
/**
@@ -116,7 +119,9 @@ namespace __gnu_debug
*/
class _Safe_unordered_container_base : public _Safe_sequence_base
{
+ friend class _Safe_local_iterator_base;
typedef _Safe_sequence_base _Base;
+
public:
/// The list of mutable local iterators that reference this container
_Safe_iterator_base* _M_local_iterators;
@@ -158,7 +163,7 @@ namespace __gnu_debug
void
_M_swap(_Safe_unordered_container_base& __x) noexcept;
- public:
+ private:
/** Attach an iterator to this container. */
void
_M_attach_local(_Safe_iterator_base* __it, bool __constant);
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index ed0417a..6b0db11 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -30,12 +30,13 @@
#include <debug/safe_unordered_base.h>
#include <debug/safe_iterator.h>
#include <debug/safe_local_iterator.h>
+#include <debug/vector>
#include <cassert>
#include <cstdio>
+#include <cctype> // for std::isspace
#include <algorithm> // for std::min
-#include <functional> // for _Hash_impl
#include <cxxabi.h> // for __cxa_demangle
@@ -47,11 +48,16 @@ namespace
* in order to limit contention without breaking current library binary
* compatibility. */
__gnu_cxx::__mutex&
- get_safe_base_mutex(void* __address)
+ get_safe_base_mutex(void* address)
{
const size_t mask = 0xf;
static __gnu_cxx::__mutex safe_base_mutex[mask + 1];
- const size_t index = _Hash_impl::hash(__address) & mask;
+
+ // Use arbitrarily __gnu_debug::vector<int> as the container giving
+ // alignment of debug containers.
+ const auto alignbits = __builtin_ctz(alignof(__gnu_debug::vector<int>));
+ const size_t index
+ = (reinterpret_cast<std::size_t>(address) >> alignbits) & mask;
return safe_base_mutex[index];
}
@@ -70,8 +76,8 @@ namespace
}
void
- swap_seq(__gnu_debug::_Safe_sequence_base& __lhs,
- __gnu_debug::_Safe_sequence_base& __rhs)
+ swap_seq_single(__gnu_debug::_Safe_sequence_base& __lhs,
+ __gnu_debug::_Safe_sequence_base& __rhs)
{
swap(__lhs._M_version, __rhs._M_version);
swap_its(__lhs, __lhs._M_iterators,
@@ -80,11 +86,42 @@ namespace
__rhs, __rhs._M_const_iterators);
}
+ template<typename _Action>
+ void
+ lock_and_run(__gnu_cxx::__mutex& lhs_mutex, __gnu_cxx::__mutex& rhs_mutex,
+ _Action action)
+ {
+ // We need to lock both sequences to run action.
+ if (&lhs_mutex == &rhs_mutex)
+ {
+ __gnu_cxx::__scoped_lock sentry(lhs_mutex);
+ action();
+ }
+ else
+ {
+ __gnu_cxx::__scoped_lock sentry1(&lhs_mutex < &rhs_mutex
+ ? lhs_mutex : rhs_mutex);
+ __gnu_cxx::__scoped_lock sentry2(&lhs_mutex < &rhs_mutex
+ ? rhs_mutex : lhs_mutex);
+ action();
+ }
+ }
+
+ void
+ swap_seq(__gnu_cxx::__mutex& lhs_mutex,
+ __gnu_debug::_Safe_sequence_base& lhs,
+ __gnu_cxx::__mutex& rhs_mutex,
+ __gnu_debug::_Safe_sequence_base& rhs)
+ {
+ lock_and_run(lhs_mutex, rhs_mutex,
+ [&lhs, &rhs]() { swap_seq_single(lhs, rhs); });
+ }
+
void
- swap_ucont(__gnu_debug::_Safe_unordered_container_base& __lhs,
- __gnu_debug::_Safe_unordered_container_base& __rhs)
+ swap_ucont_single(__gnu_debug::_Safe_unordered_container_base& __lhs,
+ __gnu_debug::_Safe_unordered_container_base& __rhs)
{
- swap_seq(__lhs, __rhs);
+ swap_seq_single(__lhs, __rhs);
swap_its(__lhs, __lhs._M_local_iterators,
__rhs, __rhs._M_local_iterators);
swap_its(__lhs, __lhs._M_const_local_iterators,
@@ -92,6 +129,16 @@ namespace
}
void
+ swap_ucont(__gnu_cxx::__mutex& lhs_mutex,
+ __gnu_debug::_Safe_unordered_container_base& lhs,
+ __gnu_cxx::__mutex& rhs_mutex,
+ __gnu_debug::_Safe_unordered_container_base& rhs)
+ {
+ lock_and_run(lhs_mutex, rhs_mutex,
+ [&lhs, &rhs]() { swap_ucont_single(lhs, rhs); });
+ }
+
+ void
detach_all(__gnu_debug::_Safe_iterator_base* __iter)
{
for (; __iter;)
@@ -248,25 +295,7 @@ namespace __gnu_debug
void
_Safe_sequence_base::
_M_swap(_Safe_sequence_base& __x) noexcept
- {
- // We need to lock both sequences to swap
- using namespace __gnu_cxx;
- __mutex *__this_mutex = &_M_get_mutex();
- __mutex *__x_mutex = &__x._M_get_mutex();
- if (__this_mutex == __x_mutex)
- {
- __scoped_lock __lock(*__this_mutex);
- swap_seq(*this, __x);
- }
- else
- {
- __scoped_lock __l1(__this_mutex < __x_mutex
- ? *__this_mutex : *__x_mutex);
- __scoped_lock __l2(__this_mutex < __x_mutex
- ? *__x_mutex : *__this_mutex);
- swap_seq(*this, __x);
- }
- }
+ { swap_seq(_M_get_mutex(), *this, __x._M_get_mutex(), __x); }
__gnu_cxx::__mutex&
_Safe_sequence_base::
@@ -390,7 +419,7 @@ namespace __gnu_debug
__gnu_cxx::__mutex&
_Safe_iterator_base::
_M_get_mutex() throw ()
- { return get_safe_base_mutex(_M_sequence); }
+ { return _M_sequence->_M_get_mutex(); }
_Safe_unordered_container_base*
_Safe_local_iterator_base::
@@ -468,25 +497,7 @@ namespace __gnu_debug
void
_Safe_unordered_container_base::
_M_swap(_Safe_unordered_container_base& __x) noexcept
- {
- // We need to lock both containers to swap
- using namespace __gnu_cxx;
- __mutex *__this_mutex = &_M_get_mutex();
- __mutex *__x_mutex = &__x._M_get_mutex();
- if (__this_mutex == __x_mutex)
- {
- __scoped_lock __lock(*__this_mutex);
- swap_ucont(*this, __x);
- }
- else
- {
- __scoped_lock __l1(__this_mutex < __x_mutex
- ? *__this_mutex : *__x_mutex);
- __scoped_lock __l2(__this_mutex < __x_mutex
- ? *__x_mutex : *__this_mutex);
- swap_ucont(*this, __x);
- }
- }
+ { swap_ucont(_M_get_mutex(), *this, __x._M_get_mutex(), __x); }
void
_Safe_unordered_container_base::
diff --git a/libstdc++-v3/testsuite/23_containers/vector/debug/mutex_association.cc b/libstdc++-v3/testsuite/23_containers/vector/debug/mutex_association.cc
new file mode 100644
index 0000000..a3c56e2
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/debug/mutex_association.cc
@@ -0,0 +1,42 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+//
+
+#include <set>
+#include <debug/vector>
+
+#include <testsuite_hooks.h>
+
+class container : public __gnu_debug::_Safe_sequence<container>
+{
+public:
+ __gnu_cxx::__mutex&
+ get_mutex()
+ { return this->_M_get_mutex(); }
+};
+
+int
+main()
+{
+ std::set<__gnu_cxx::__mutex*> mutexes;
+ container conts[17];
+
+ for (int i = 0; i != 16; ++i)
+ VERIFY( mutexes.insert(&conts[i].get_mutex()).second );
+
+ VERIFY( !mutexes.insert(&conts[16].get_mutex()).second );
+}
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-19 20:19 ` François Dumont
@ 2016-09-20 8:58 ` Jonathan Wakely
2016-09-27 15:39 ` Jonathan Wakely
2016-09-20 9:20 ` Jonathan Wakely
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Jonathan Wakely @ 2016-09-20 8:58 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 19/09/16 21:56 +0200, François Dumont wrote:
>Hi
>
> Following our conversation here is a much simpler patch. I just
>consider that all debug containers will have the same alignment.
>
> Even if I submit this patch as a whole I will commit into pieces,
>at least one for the pure cleanup parts and one for the debug.cc
>change.
>
> Among those changes there is:
>- __gnu_cxx::__scoped_lock(this->_M_get_mutex());
>+ __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
>
> I would appreciate if you could tell me what was happening with
>the initial expression. I don't understand why it is compiling. I even
>tried the same in debug.cc and started having compilation errors.
It creates a temporary __scoped_lock, which immediately goes out of
scope and unlocks the mutex again. This should be fixed on the gcc-5
and gcc-6 branches too.
I'll review the rest of the patch today.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-19 20:19 ` François Dumont
2016-09-20 8:58 ` Jonathan Wakely
@ 2016-09-20 9:20 ` Jonathan Wakely
2016-09-26 11:55 ` Jonathan Wakely
2016-09-26 12:23 ` Andreas Schwab
2017-12-29 16:10 ` Andreas Schwab
3 siblings, 1 reply; 16+ messages in thread
From: Jonathan Wakely @ 2016-09-20 9:20 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 19/09/16 21:56 +0200, François Dumont wrote:
>Hi
>
> Following our conversation here is a much simpler patch. I just
>consider that all debug containers will have the same alignment.
>
> Even if I submit this patch as a whole I will commit into pieces,
>at least one for the pure cleanup parts and one for the debug.cc
>change.
>
> Among those changes there is:
>- __gnu_cxx::__scoped_lock(this->_M_get_mutex());
>+ __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
>
> I would appreciate if you could tell me what was happening with
>the initial expression. I don't understand why it is compiling. I even
>tried the same in debug.cc and started having compilation errors.
>
> * include/debug/bitset (bitset::reference::reference(const _Base_ref&,
> bitset*)): Remove __unused__ attribute.
> * include/debug/safe_base.h (_Safe_iterator_base): Make
> _Safe_sequence_base a friend.
> (_Safe_iterator_base::_M_attach): Make protected.
> (_Safe_iterator_base::_M_attach_single): Likewise.
> (_Safe_iterator_base::_M_detach): Likewise.
> (_Safe_iterator_base::_M_detach_single): Likewise.
> (_Safe_sequence_base): Make _Safe_iterator_base a friend.
>(_Safe_sequence_base::_Safe_sequence_base(_Safe_sequence_base&&)): New.
> (_Safe_sequence_base::_M_swap): Make protected.
> (_Safe_sequence_base::_M_attach): Make private.
> (_Safe_sequence_base::_M_attach_single): Likewise.
> (_Safe_sequence_base::_M_detach): Likewise.
> (_Safe_sequence_base::_M_detach_single): Likewise.
> * include/debug/safe_container.h
> (_Safe_container::_Safe_container(_Safe_container&&)): Make default.
> * include/debug/safe_iterator.h
> (_Safe_iterator::operator++()): Name __scoped_lock instance.
> * include/debug/safe_iterator.tcc: Remove trailing line.
> * include/debug/safe_unordered_base.h
> (_Safe_local_iterator_base::_M_attach): Make protected.
> (_Safe_local_iterator_base::_M_attach_single): Likewise.
> (_Safe_local_iterator_base::_M_detach): Likewise.
> (_Safe_local_iterator_base::_M_detach_single): Likewise.
> (_Safe_unordered_container_base): Make _Safe_local_iterator_base
>friend.
> (_Safe_unordered_container_base::_M_attach_local): Make private.
> (_Safe_unordered_container_base::_M_attach_local_single): Likewise.
> (_Safe_unordered_container_base::_M_detach_local): Likewise.
> (_Safe_unordered_container_base::_M_detach_local_single): Likewise.
> * src/c++11/debug.cc: Include debug/vector. Include cctype. Remove
> functional.
> (get_safe_base_mutex): Get mutex based on address lowest non nil bits.
> * testsuite/23_containers/vector/debug/mutex_association.cc: New.
>
>Tested under Linux x86_64.
>
>Ok to commit ?
Yes, OK for trunk. Thanks for revising it, I think this is a much
bettter fix.
>On 15/09/2016 10:51, Jonathan Wakely wrote:
>>N.B. https://gcc.gnu.org/PR71312
>>>Ok, debug mode is also impacted. Shouldn't the alignment be set on
>>>the __mutex type directly ?
>>
>>No, definitely not. Apart from the fact that it would be an ABI
>>change, it would mean that struct X { __mutex mx; int i; } would not
>>place the int right next to the mutex, it would force it onto a
>>different cacheline. We want our arrays of mutexes to be on separate
>>cachelines, but most uses of __mutex are not in an array.
>>
>>We probably can't fix this properly yet, because we don't have the
>>hardware_destructive_interference_size value. We could just make a
>>conservative estimate of 64 bytes though.
>Thanks for explaining that it is to avoid false sharing.
>
>Maybe we could share this mutex pool with debug mode. This way the day
>we fix this false sharing issue it will benefit to both shared_ptr and
>debug mode.
Yes, that would reduce the size of the library, by not having two
arrays of mutexes. Currently the arrays are not visible outside their
own file, but that's not too hard to solve.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-20 9:20 ` Jonathan Wakely
@ 2016-09-26 11:55 ` Jonathan Wakely
0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Wakely @ 2016-09-26 11:55 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
On 20/09/16 09:57 +0100, Jonathan Wakely wrote:
>On 19/09/16 21:56 +0200, François Dumont wrote:
>>Hi
>>
>> Following our conversation here is a much simpler patch. I just
>>consider that all debug containers will have the same alignment.
>>
>> Even if I submit this patch as a whole I will commit into pieces,
>>at least one for the pure cleanup parts and one for the debug.cc
>>change.
>>
>> Among those changes there is:
>>- __gnu_cxx::__scoped_lock(this->_M_get_mutex());
>>+ __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
>>
>> I would appreciate if you could tell me what was happening with
>>the initial expression. I don't understand why it is compiling. I
>>even tried the same in debug.cc and started having compilation
>>errors.
>>
>> * include/debug/bitset (bitset::reference::reference(const _Base_ref&,
>> bitset*)): Remove __unused__ attribute.
>> * include/debug/safe_base.h (_Safe_iterator_base): Make
>> _Safe_sequence_base a friend.
>> (_Safe_iterator_base::_M_attach): Make protected.
>> (_Safe_iterator_base::_M_attach_single): Likewise.
>> (_Safe_iterator_base::_M_detach): Likewise.
>> (_Safe_iterator_base::_M_detach_single): Likewise.
>> (_Safe_sequence_base): Make _Safe_iterator_base a friend.
>>(_Safe_sequence_base::_Safe_sequence_base(_Safe_sequence_base&&)): New.
>> (_Safe_sequence_base::_M_swap): Make protected.
>> (_Safe_sequence_base::_M_attach): Make private.
>> (_Safe_sequence_base::_M_attach_single): Likewise.
>> (_Safe_sequence_base::_M_detach): Likewise.
>> (_Safe_sequence_base::_M_detach_single): Likewise.
>> * include/debug/safe_container.h
>> (_Safe_container::_Safe_container(_Safe_container&&)): Make default.
>> * include/debug/safe_iterator.h
>> (_Safe_iterator::operator++()): Name __scoped_lock instance.
>> * include/debug/safe_iterator.tcc: Remove trailing line.
>> * include/debug/safe_unordered_base.h
>> (_Safe_local_iterator_base::_M_attach): Make protected.
>> (_Safe_local_iterator_base::_M_attach_single): Likewise.
>> (_Safe_local_iterator_base::_M_detach): Likewise.
>> (_Safe_local_iterator_base::_M_detach_single): Likewise.
>> (_Safe_unordered_container_base): Make _Safe_local_iterator_base
>>friend.
>> (_Safe_unordered_container_base::_M_attach_local): Make private.
>> (_Safe_unordered_container_base::_M_attach_local_single): Likewise.
>> (_Safe_unordered_container_base::_M_detach_local): Likewise.
>> (_Safe_unordered_container_base::_M_detach_local_single): Likewise.
>> * src/c++11/debug.cc: Include debug/vector. Include cctype. Remove
>> functional.
>> (get_safe_base_mutex): Get mutex based on address lowest non nil bits.
>> * testsuite/23_containers/vector/debug/mutex_association.cc: New.
>>
>>Tested under Linux x86_64.
>>
>>Ok to commit ?
>
>Yes, OK for trunk. Thanks for revising it, I think this is a much
>bettter fix.
This caused a new FAIL:
FAIL: 23_containers/list/debug/invalidation/4.cc (test for excess errors)
UNRESOLVED: 23_containers/list/debug/invalidation/4.cc compilation failed to produce executable
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-19 20:19 ` François Dumont
2016-09-20 8:58 ` Jonathan Wakely
2016-09-20 9:20 ` Jonathan Wakely
@ 2016-09-26 12:23 ` Andreas Schwab
2016-09-26 22:09 ` François Dumont
2017-12-29 16:10 ` Andreas Schwab
3 siblings, 1 reply; 16+ messages in thread
From: Andreas Schwab @ 2016-09-26 12:23 UTC (permalink / raw)
To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc-patches
FAIL: 23_containers/list/debug/invalidation/4.cc (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20160926/Build/m68k-linux/libstdc++-v3/include/debug/safe_sequence.tcc:89: error: 'void __gnu_debug::_Safe_iterator_base::_M_detach_single()' is protected within this context
/daten/aranym/gcc/gcc-20160926/Build/m68k-linux/libstdc++-v3/include/debug/safe_sequence.tcc:112: error: 'void __gnu_debug::_Safe_iterator_base::_M_detach_single()' is protected within this context
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-26 12:23 ` Andreas Schwab
@ 2016-09-26 22:09 ` François Dumont
2016-09-27 10:32 ` Jonathan Wakely
0 siblings, 1 reply; 16+ messages in thread
From: François Dumont @ 2016-09-26 22:09 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Jonathan Wakely, libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 591 bytes --]
Fixed with attached patch.
François
On 26/09/2016 13:56, Andreas Schwab wrote:
> FAIL: 23_containers/list/debug/invalidation/4.cc (test for excess errors)
> Excess errors:
> /daten/aranym/gcc/gcc-20160926/Build/m68k-linux/libstdc++-v3/include/debug/safe_sequence.tcc:89: error: 'void __gnu_debug::_Safe_iterator_base::_M_detach_single()' is protected within this context
> /daten/aranym/gcc/gcc-20160926/Build/m68k-linux/libstdc++-v3/include/debug/safe_sequence.tcc:112: error: 'void __gnu_debug::_Safe_iterator_base::_M_detach_single()' is protected within this context
>
> Andreas.
>
[-- Attachment #2: safe_base.h.patch --]
[-- Type: text/x-patch, Size: 492 bytes --]
Index: include/debug/safe_base.h
===================================================================
--- include/debug/safe_base.h (revision 240509)
+++ include/debug/safe_base.h (working copy)
@@ -121,11 +121,11 @@
void
_M_detach();
+ public:
/** Likewise, but not thread-safe. */
void
_M_detach_single() throw ();
- public:
/** Determines if we are attached to the given sequence. */
bool
_M_attached_to(const _Safe_sequence_base* __seq) const
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-26 22:09 ` François Dumont
@ 2016-09-27 10:32 ` Jonathan Wakely
2016-09-28 19:35 ` François Dumont
0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Wakely @ 2016-09-27 10:32 UTC (permalink / raw)
To: François Dumont; +Cc: Andreas Schwab, libstdc++, gcc-patches
On 26/09/16 22:36 +0200, François Dumont wrote:
>Fixed with attached patch.
>
>François
>
>
>On 26/09/2016 13:56, Andreas Schwab wrote:
>>FAIL: 23_containers/list/debug/invalidation/4.cc (test for excess errors)
>>Excess errors:
>>/daten/aranym/gcc/gcc-20160926/Build/m68k-linux/libstdc++-v3/include/debug/safe_sequence.tcc:89: error: 'void __gnu_debug::_Safe_iterator_base::_M_detach_single()' is protected within this context
>>/daten/aranym/gcc/gcc-20160926/Build/m68k-linux/libstdc++-v3/include/debug/safe_sequence.tcc:112: error: 'void __gnu_debug::_Safe_iterator_base::_M_detach_single()' is protected within this context
>>
>>Andreas.
>>
>
>Index: include/debug/safe_base.h
>===================================================================
>--- include/debug/safe_base.h (revision 240509)
>+++ include/debug/safe_base.h (working copy)
>@@ -121,11 +121,11 @@
> void
> _M_detach();
>
>+ public:
> /** Likewise, but not thread-safe. */
> void
> _M_detach_single() throw ();
>
>- public:
> /** Determines if we are attached to the given sequence. */
> bool
> _M_attached_to(const _Safe_sequence_base* __seq) const
Would this be a smaller change, that doesn't make the member
accessible to all code?
--- a/libstdc++-v3/include/debug/safe_base.h
+++ b/libstdc++-v3/include/debug/safe_base.h
@@ -50,6 +50,7 @@ namespace __gnu_debug
class _Safe_iterator_base
{
friend class _Safe_sequence_base;
+ template<typename> friend class _Safe_sequence;
public:
/** The sequence this iterator references; may be NULL to indicate
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-20 8:58 ` Jonathan Wakely
@ 2016-09-27 15:39 ` Jonathan Wakely
2016-09-27 21:14 ` François Dumont
0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Wakely @ 2016-09-27 15:39 UTC (permalink / raw)
To: François Dumont; +Cc: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 994 bytes --]
On 20/09/16 09:53 +0100, Jonathan Wakely wrote:
>On 19/09/16 21:56 +0200, François Dumont wrote:
>>Hi
>>
>> Following our conversation here is a much simpler patch. I just
>>consider that all debug containers will have the same alignment.
>>
>> Even if I submit this patch as a whole I will commit into pieces,
>>at least one for the pure cleanup parts and one for the debug.cc
>>change.
>>
>> Among those changes there is:
>>- __gnu_cxx::__scoped_lock(this->_M_get_mutex());
>>+ __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
>>
>> I would appreciate if you could tell me what was happening with
>>the initial expression. I don't understand why it is compiling. I
>>even tried the same in debug.cc and started having compilation
>>errors.
>
>It creates a temporary __scoped_lock, which immediately goes out of
>scope and unlocks the mutex again. This should be fixed on the gcc-5
>and gcc-6 branches too.
I'm committing this to the gcc-5 and gcc-6 branches.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 822 bytes --]
commit 5218b515fdb54881d8a2b87c28eb5fd84c52db01
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Tue Sep 27 16:22:28 2016 +0100
Fix lifetime of mutex lock in debug iterator
* include/debug/safe_iterator.h (_Safe_iterator::operator++()): Fix
lifetime of lock.
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index 5368f3b..3f5a7f8 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -296,7 +296,7 @@ namespace __gnu_debug
_GLIBCXX_DEBUG_VERIFY(this->_M_incrementable(),
_M_message(__msg_bad_inc)
._M_iterator(*this, "this"));
- __gnu_cxx::__scoped_lock(this->_M_get_mutex());
+ __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
++base();
return *this;
}
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-27 15:39 ` Jonathan Wakely
@ 2016-09-27 21:14 ` François Dumont
0 siblings, 0 replies; 16+ messages in thread
From: François Dumont @ 2016-09-27 21:14 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
On 27/09/2016 17:29, Jonathan Wakely wrote:
> On 20/09/16 09:53 +0100, Jonathan Wakely wrote:
>> On 19/09/16 21:56 +0200, François Dumont wrote:
>>> Hi
>>>
>>> Following our conversation here is a much simpler patch. I just
>>> consider that all debug containers will have the same alignment.
>>>
>>> Even if I submit this patch as a whole I will commit into pieces,
>>> at least one for the pure cleanup parts and one for the debug.cc
>>> change.
>>>
>>> Among those changes there is:
>>> - __gnu_cxx::__scoped_lock(this->_M_get_mutex());
>>> + __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
>>>
>>> I would appreciate if you could tell me what was happening with
>>> the initial expression. I don't understand why it is compiling. I
>>> even tried the same in debug.cc and started having compilation errors.
>>
>> It creates a temporary __scoped_lock, which immediately goes out of
>> scope and unlocks the mutex again. This should be fixed on the gcc-5
>> and gcc-6 branches too.
>
> I'm committing this to the gcc-5 and gcc-6 branches.
>
>
I still had plan to do so but if you have those branches under your
hands I appreciate.
Thanks,
François
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-27 10:32 ` Jonathan Wakely
@ 2016-09-28 19:35 ` François Dumont
2016-09-28 19:47 ` Jonathan Wakely
0 siblings, 1 reply; 16+ messages in thread
From: François Dumont @ 2016-09-28 19:35 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Andreas Schwab, libstdc++, gcc-patches
On 27/09/2016 12:32, Jonathan Wakely wrote:
> Index: include/debug/safe_base.h
>> ===================================================================
>> --- include/debug/safe_base.h (revision 240509)
>> +++ include/debug/safe_base.h (working copy)
>> @@ -121,11 +121,11 @@
>> void
>> _M_detach();
>>
>> + public:
>> /** Likewise, but not thread-safe. */
>> void
>> _M_detach_single() throw ();
>>
>> - public:
>> /** Determines if we are attached to the given sequence. */
>> bool
>> _M_attached_to(const _Safe_sequence_base* __seq) const
>
>
> Would this be a smaller change, that doesn't make the member
> accessible to all code?
>
> --- a/libstdc++-v3/include/debug/safe_base.h
> +++ b/libstdc++-v3/include/debug/safe_base.h
> @@ -50,6 +50,7 @@ namespace __gnu_debug
> class _Safe_iterator_base
> {
> friend class _Safe_sequence_base;
> + template<typename> friend class _Safe_sequence;
>
> public:
> /** The sequence this iterator references; may be NULL to indicate
> .
>
I am not a great fan of friend class. As long as it was friend
declaration between iterator and sequence base types it was ok. Now that
we need to make a template class friend I consider that it is too much
friendship and prefer to make the necessary method public.
But if you think otherwise just tell me and I will use your approach.
François
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-28 19:35 ` François Dumont
@ 2016-09-28 19:47 ` Jonathan Wakely
0 siblings, 0 replies; 16+ messages in thread
From: Jonathan Wakely @ 2016-09-28 19:47 UTC (permalink / raw)
To: François Dumont; +Cc: Andreas Schwab, libstdc++, gcc-patches
On 28/09/16 21:30 +0200, François Dumont wrote:
>On 27/09/2016 12:32, Jonathan Wakely wrote:
>>Index: include/debug/safe_base.h
>>>===================================================================
>>>--- include/debug/safe_base.h (revision 240509)
>>>+++ include/debug/safe_base.h (working copy)
>>>@@ -121,11 +121,11 @@
>>> void
>>> _M_detach();
>>>
>>>+ public:
>>> /** Likewise, but not thread-safe. */
>>> void
>>> _M_detach_single() throw ();
>>>
>>>- public:
>>> /** Determines if we are attached to the given sequence. */
>>> bool
>>> _M_attached_to(const _Safe_sequence_base* __seq) const
>>
>>
>>Would this be a smaller change, that doesn't make the member
>>accessible to all code?
>>
>>--- a/libstdc++-v3/include/debug/safe_base.h
>>+++ b/libstdc++-v3/include/debug/safe_base.h
>>@@ -50,6 +50,7 @@ namespace __gnu_debug
>> class _Safe_iterator_base
>> {
>> friend class _Safe_sequence_base;
>>+ template<typename> friend class _Safe_sequence;
>>
>> public:
>> /** The sequence this iterator references; may be NULL to indicate
>>.
>>
>I am not a great fan of friend class. As long as it was friend
>declaration between iterator and sequence base types it was ok. Now
>that we need to make a template class friend I consider that it is too
>much friendship and prefer to make the necessary method public.
OK.
>But if you think otherwise just tell me and I will use your approach.
Making it public is fine. Please commit your original patch, thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: debug container mutex association
2016-09-19 20:19 ` François Dumont
` (2 preceding siblings ...)
2016-09-26 12:23 ` Andreas Schwab
@ 2017-12-29 16:10 ` Andreas Schwab
3 siblings, 0 replies; 16+ messages in thread
From: Andreas Schwab @ 2017-12-29 16:10 UTC (permalink / raw)
To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc-patches
On Sep 19 2016, François Dumont <frs.dumont@gmail.com> wrote:
> diff --git a/libstdc++-v3/testsuite/23_containers/vector/debug/mutex_association.cc b/libstdc++-v3/testsuite/23_containers/vector/debug/mutex_association.cc
> new file mode 100644
> index 0000000..a3c56e2
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/23_containers/vector/debug/mutex_association.cc
> @@ -0,0 +1,42 @@
> +// Copyright (C) 2016 Free Software Foundation, Inc.
> +//
> +// This file is part of the GNU ISO C++ Library. This library is free
> +// software; you can redistribute it and/or modify it under the
> +// terms of the GNU General Public License as published by the
> +// Free Software Foundation; either version 3, or (at your option)
> +// any later version.
> +//
> +// This library is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +// GNU General Public License for more details.
> +//
> +// You should have received a copy of the GNU General Public License along
> +// with this library; see the file COPYING3. If not see
> +// <http://www.gnu.org/licenses/>.
> +//
> +
> +#include <set>
> +#include <debug/vector>
> +
> +#include <testsuite_hooks.h>
> +
> +class container : public __gnu_debug::_Safe_sequence<container>
> +{
> +public:
> + __gnu_cxx::__mutex&
> + get_mutex()
> + { return this->_M_get_mutex(); }
> +};
> +
> +int
> +main()
> +{
> + std::set<__gnu_cxx::__mutex*> mutexes;
> + container conts[17];
> +
> + for (int i = 0; i != 16; ++i)
> + VERIFY( mutexes.insert(&conts[i].get_mutex()).second );
There will be less than 16 unique mutexes, if sizeof(container) has more
trailing zero bits than alignof(__gnu_debug::vector<int>).
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-12-29 16:10 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-13 20:37 debug container mutex association François Dumont
2016-09-14 9:01 ` Jonathan Wakely
2016-09-14 20:53 ` François Dumont
2016-09-15 9:15 ` Jonathan Wakely
2016-09-19 20:19 ` François Dumont
2016-09-20 8:58 ` Jonathan Wakely
2016-09-27 15:39 ` Jonathan Wakely
2016-09-27 21:14 ` François Dumont
2016-09-20 9:20 ` Jonathan Wakely
2016-09-26 11:55 ` Jonathan Wakely
2016-09-26 12:23 ` Andreas Schwab
2016-09-26 22:09 ` François Dumont
2016-09-27 10:32 ` Jonathan Wakely
2016-09-28 19:35 ` François Dumont
2016-09-28 19:47 ` Jonathan Wakely
2017-12-29 16:10 ` Andreas Schwab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).