You are locking each
push_back to the vectors defined as local variables. How can other threads be manipulating the same vectors?
It's also using a global mutex for all threads, which will pretty much destroy any threading benefit.
Use a scoped lock rather than direct
Don't repeat the whole thing when only a variable changes: Break out the choice of
rvec in the condition, and then have one copy of the expression that uses it. E.g.
auto& side= i<=mid ? lvec : rvec;
But looking at that when I typed it, I realized that you are choosing based on
i not the value (
i is just counting up. So you should just copy the first half of
left as one statement and copy the other half to
right as one statement. That can be further improved: if you copy off
right to another vector, then just call
vec.resize to turn it into what you wanted in
left, with no copying needed.
Your leftover is doing the same thing: just copy the entire rest of
rvec with a single call each. Don't write a loop.
Your create two new threads and then immediately join them. That means that the original thread is doing nothing, and is just wasted overhead of having created it. You have two parallel things to do; put half in a new thread and do the other half in this thread. This will have the identical effect, but without the overhead of more thread creation/joining.
Finally, nobody ever sorts a simple vector of
int. Make your code useable for real work by making it a template argument. You can use
int in your testing, but the code is ready to use with other types.
Also, if you look at the standard algorithms,
stable_sort, etc. are defined to take iterators, and are not limited to
std::vector. You are passing in index positions (as
int, not as the type that
vector actually uses for indexing) to allow sorting part of a
vector, as this is needed by your recursive approach. But it works more naturally if you just used begin/end iterators, and don't need to pass the collection at all.
That is, make the signature look like
I suggest you review what's in the algorithm header so you're familiar with what's in there, and can reach for those instead of writing loops from scratch for simple operations.