This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: debug mode performance patch


On 7 November 2010 15:36, Paolo Carlini wrote:
> On 11/04/2010 09:27 PM, François Dumont wrote:
>> Moreover I forget to signal that in _Safe_sequence_base::_M_swap I had
>> to lock both sequences mutex. So I hope that the scope_lock are
>> reentrant cause for the moment both sequences might be associated to
>> the same mutex instance. I hope I haven't been simply lucky during tests.
> Before we go ahead with the patch could you please explain the above
> issue in better detail? Maybe Jon could also help here.

It isn't safe to lock the same mutex twice like that, you'd need to
use recursive_mutex.

But that would still be unsafe, _M_swap can deadlock if swap(x, y) and
swap(y, x) are called concurrently, because the same two mutexes will
be locked in different orders. You need to enforce an ordering to the
locks, locking the one with the lower address first would be the
obvious choice since you know they're part of the same array and their
addresses are comparable.

Untested:

  namespace
  {
    void
    __do_swap(_Safe_sequence_base& __x, _Safe_sequence_base& __y)
    {
      swap(__x._M_iterators, __y._M_iterators);
      swap(__x._M_const_iterators, __y._M_const_iterators);
      swap(__x._M_version, __y._M_version);
      _Safe_iterator_base* __iter;
      for (__iter = __x._M_iterators; __iter; __iter = __iter->_M_next)
        __iter->_M_sequence = &__x;
      for (__iter = __y._M_iterators; __iter; __iter = __iter->_M_next)
        __iter->_M_sequence = &__y;
      for (__iter = __x._M_const_iterators; __iter; __iter = __iter->_M_next)
        __iter->_M_sequence = &__x;
      for (__iter = __y._M_const_iterators; __iter; __iter = __iter->_M_next)
        __iter->_M_sequence = &__y;
    }
  }

  void
  _Safe_sequence_base::
  _M_swap(_Safe_sequence_base& __x)
  {
    __gnu_cxx::__mutex& __mx1 = _M_get_mutex();
    __gnu_cxx::__mutex& __mx2 = __x._M_get_mutex();
    if (&__mx1 == &__mx2)
    {
      __gnu_cxx::__scoped_lock __sentry(__mx1);
      __do_swap(*this, __x);
    }
    else
    {
      __gnu_cxx::__scoped_lock __sentry1(&__mx1 < & __mx2 ? __mx1 : __mx2);
      __gnu_cxx::__scoped_lock __sentry2(&__mx1 < & __mx2 ? __mx2 : __mx1);
      __do_swap(*this, __x);
    }
  }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]