<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 6, 2014 at 8:52 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron.ballman@gmail.com" target="_blank">aaron.ballman@gmail.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

I'm slightly concerned about the overhead from making these functions<br>
virtual. Do you happen to have any indication as to how this affects<br>
performance? What was wrong with the previous design where the base<br>
did not hold concrete data, but instead a void *?</blockquote><div><br></div><div>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.  </div>
<div><br></div><div>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.</div>
<div><br></div><div>I will work on addressing the other issues you pointed out, let me know your thoughts on the rationale above.</div><div><br></div><div>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.</div>
<div><br></div><div>Thanks!</div><div>Zach </div></div></div></div>