On Tue, Mar 13, 2018 at 8:35 AM, Sage Weil <sweil@xxxxxxxxxx> wrote: > Radoslaw noticed that the nlock field in common/Mutex.h was slowing us > down by 5% (and was basically useless). Also, all of the conditional > behavior based on constructor argumenst were fixed at compile time. > > This points (again) at the fact that we have our own mutex wrapper that > we're using instead of std::mutex. Chatted a bit with Josh and here is > our thinking: > > - Someday we could replace our lockdep with helgrind, but there is a lot > of work needed to get there. > > - Right now, our lockdep provides a lot of value because it is enabled by > default with vstart clusters, so we catch most ordering issues there. (We > also have some teuthology tests that enable it.) > > - We could convert our Mutex class to conform to the c++ Mutex concept. > This mainly means renaming Lock -> lock, TryLock -> try_lock, Unlock -> > unlock. That would let us use things like std::unique_lock<> and > std::lock_guard<> with it. Cond would need a similar makeover. > > - We can rename it (or alias it) to something like debug_mutex. > > - We can alias ceph::mutex to either std::mutex or ceph::debug_mutex at > compile time. Production builds can exclude our lockdep and use > std::mutex exclusively. Default dev/vstart builds can use debug_mutex. > Maybe notcmalloc flavor uses debug_mutex and default flavor builds use > std::mutex, so that we have both available for teuthology tests. We don't actually have that many notcmalloc tests in the suite (they are slower!); I presume we should run with our lockdep enabled more often than not to catch the rarer lock inversions. Otherwise, this sounds like a fine idea to me. Trying to unify these behaviors from a code style perspective is long overdue given our other modernization efforts. :) -Greg > > - All of our existing code can be converted to use ceph::mutex and > ceph::condition_variable instead of Mutex and Cond. > > - Profit! > > How does that sound? > > sage > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
