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 11/10/2010 12:03 AM, Jonathan Wakely wrote:
On 9 November 2010 20:53, François Dumont wrote:
As announced I prefer to use a consistent approach in the debug mode locks,
I always lock the sequence mutex as soon as I start modifying the lists of
safe iterators within it. In swap we touch to both sequences' lists at the
same time so we have to lock both mutexes. However I realized that if you
have a thread iterating over a sequence while you swap it in an other thread
it has chances to fail in debug mode because when the thread iterating will
check for sequence end it might do so at a moment where both safe iterators
has not been assigned to the same safe sequence resulting in an invalid
comparison of iterators from different sequences. Now the question is
whether this kind of concurrent usage should be considered as valid ?
It's generally not valid for users to access distinct sequences
concurrently, but your change means that distinct objects share
mutexes behind the scenes, even if users don't share sequences.
Well, this is not something my change introduce. For the moment the debug mode is using a single mutex for all synchronizations. To keep binary compatibility I haven't been able to add a mutex instance to the safe sequence so I introduce a small mutex pool to limit contention.
There are two problems with _M_swap in your patch:

1) if two objects are swapped whih share the same mutex then it will
try to lock the same mutex twice.  You should be able to reproduce
this like so (untested):

__gnu_debug::vector<int>  v[17];
swap(v[0], v[16]);

If I'm not mistaken, v[0] and v[16] should get the same mutex from
get_mutex and so should lock it twice. That's bad, even in a
single-threaded program.
Yes, this is why I asked if mutex are guarantied to be reentrant even if I agree that even if it is so it is still bad practice.
2) consider four sequences, A, B, C and D. They happen to have
addresses such that:

&A.get_mutex() ==&C.get_mutex()
and
&B.get_mutex() ==&D.get_mutex()

now, in my multithreaded program I do this concurrently:
swap(A, B); swap(D, C);
I'm not accessing any single object in more than one thread, but it
will deadlock.
Indeed
The solution is to ensure that you don't lock the same mutex twice and
you always lock in a consistent order
Ok, so except fixing this lock issue you seem to agree on the modified debug mode lock model, great. I will fix this issue and submit a new proposition.

Thanks


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