On 03/13/2018 10:35 AM, Sage Weil 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. - 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?
I like it! :)
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
