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

[Reid Kleckner]

On Fri, Jun 6, 2014 at 9:52 AM, Zachary Turner <zturner at google.com> wrote:

> 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.
>

Whose idiomatic C++ are we talking about?  Hang around C++ compiler writers
and eventually everything becomes a template.  =P


> 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.
>

IMO the "debug" mutexes are really an attempt to optimize LLVM in
single-threaded mode (i.e. when using clang as a compiler).  They only
maintain 'acquired' so that a single-threaded run of the LLVM test suite
reveals any deadlocks that might appear in multi-threaded mode.  We should
do some svn archaeology to see if this mt_only mode is worth maintaining.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140606/4ae7ca8f/attachment.html>