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.
