On Tue, 13 Mar 2018, Sage Weil wrote:
Hi,
I've done some fairly extensive work in this direction, the start of
which you can see here:
https://github.com/chardan/ceph/tree/jfw-wip-monstrous-mutex
I started by just focusing on getting rid of the runtime-selected behavior
and distinguishing which /roles/ each Mutex was fufilling. (They are
almost all non-recursive lockdep-enabled Mutexes.)
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.
Yes, this is expensive indeed, and the motivation for my diving in there.
When I benchmark it, I also notice a substantial difference.
This points (again) at the fact that we have our own mutex wrapper that
we're using instead of std::mutex.
*Especially* if we're willing to drop lockdep support in some cases (more
in a bit, I sense!). My open question is "in which cases is lockdep
actually required"?
- Someday we could replace our lockdep with helgrind, but there is a lot
of work needed to get there.
Good idea, though!
- 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.)
Ok.
- 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.
Yes, this can happen. The support for Cond is also necessary-- and
actually the thing I seem to have messed up somehow in my current
implementation. Unfortunately, I was refocused on other priorities and
haven't had a chance to revisit it.
- We can rename it (or alias it) to something like debug_mutex.
This isn't a terrible idea, because it lets us use the type system to see
/clearly/ which role the Mutex has.
- 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.
My intuition is that this is less favorable. It's not clear in the code
what's happening, and instead requires intimate knowledge of the build
system.
- All of our existing code can be converted to use ceph::mutex and
ceph::condition_variable instead of Mutex and Cond.
I'm liking that... ;-)
- Profit!
Muhahahaaha!
How does that sound?
Good! This is all pointing in the right direction!
-Jesse
--
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
