<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jun 6, 2014 at 9:52 AM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div class="">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><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></div></div></blockquote><div><br></div><div>Whose idiomatic C++ are we talking about?  Hang around C++ compiler writers and eventually everything becomes a template.  =P</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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></div></div></blockquote><div><br></div><div>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.</div>
</div></div></div>