[PATCH] Change base mutex implementations to use STL-provided mutexes

Zachary Turner zturner at google.com
Fri Jun 6 09:52:34 PDT 2014


On Fri, Jun 6, 2014 at 8:52 AM, Aaron Ballman <aaron.ballman at gmail.com>
wrote:
>
> I'm slightly concerned about the overhead from making these functions
> virtual. Do you happen to have any indication as to how this affects
> performance? What was wrong with the previous design where the base
> did not hold concrete data, but instead a void *?


I guess I took the inverse line of thought.  Unless there's data to show
that it is a performance problem, then I preferred to use idiomatic C++.
 I'm not really a fan of type erasure in general, unless you really need it
to solve a particular sign.

Another thing is that this is a mutex.  All of the performance cost
associated with these methods are going to be that of waiting on the actual
mutex, and an extra pointer indirection should be negligible.  If there is
a case where we are locking a mutex in a tight loop, then there are bigger
issues to solve, such as moving the lock to a higher level.

I will work on addressing the other issues you pointed out, let me know
your thoughts on the rationale above.

By the way, there's another approach I could have taken, which is simply to
delete llvm::sys::Mutex entirely, and embed std::mutex and
std::recursive_mutex instances directly in the code.  We lose the Debug
versions of the mutexes, but they seem to be not used heavily, so maybe
this is ok.  I took the path of least resistance and made the semantics
identical though, so if you think this other approach is better, I could
change it.

Thanks!
Zach
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140606/8758b485/attachment.html>


More information about the llvm-commits mailing list