Thanks for the recommendation, the whole abseil library looks cool. abls::Mutex is especially interesting, since it conceptually differs from traditional mutexes. Although, this means a bit more work if we want to use it, because it won't work as a drop-in replacement.
Zoli On Tue, Oct 31, 2017 at 8:47 PM, Todd Lipcon <[email protected]> wrote: > BTW I should have linked the following in my earlier email: > https://abseil.io/about/design/mutex > > -Todd > > On Tue, Oct 31, 2017 at 10:21 AM, Todd Lipcon <[email protected]> wrote: > > > Yea, we use our own mutex which is a simple wrapper around pthreads, but > > also adds some instrumentation and debugging. > > > > We have recently been thinking about switching to absl::Mutex from > Google. > > It has adaptive spinning, faster wake up for condition waiters, and a > nice > > API which avoids missed notify bugs. It also has the same or better > > instrumentation support. > > > > If you are going to switch I'd take a look at that implementation as > well. > > > > On Oct 31, 2017 10:12 AM, "Tim Armstrong" <[email protected]> > wrote: > > > >> The main case where we still use boost::mutex is when it's pair with a > >> condition variable - we don't have any condition variable that > integrates > >> with SpinLock. > >> > >> Mutex does provide some stronger fairness guarantees, I think, but afaik > >> we > >> don't rely on that anywhere. > >> > >> On Tue, Oct 31, 2017 at 9:54 AM, Daniel Hecht <[email protected]> > >> wrote: > >> > >> > We have impala::SpinLock, which works with lock_guard, unique_lock, > etc. > >> > It's adaptive so it will block after spinning for a little bit, and > >> seems > >> > to work well in most cases. It's built on gutil, which also has some > >> > tracing we could enable. Any reason not to stick with that? > >> > > >> > I haven't looked at Kudu's implementation in a while and am not > opposed > >> to > >> > using it if there's a good reason to switch. But I think maybe we > >> should > >> > first figure out how to better share code with Kudu so that we don't > >> just > >> > continue to fork general purpose utility code (which is what we're > >> > currently doing). > >> > > >> > On Tue, Oct 31, 2017 at 9:47 AM, Zoltan Borok-Nagy < > >> > [email protected]> > >> > wrote: > >> > > >> > > Hi Everyone, > >> > > > >> > > I started to review the usage of synchronization primitives in > Impala > >> and > >> > > created this JIRA issue: https://issues.apache.org/ > >> > jira/browse/IMPALA-6125 > >> > > > >> > > After some chat with Tim, we talked about the possibilities of > >> reducing > >> > the > >> > > dependence of boost. Since now we are using C++11, there are > >> std::thread, > >> > > std::mutex, and std::condition_variable in the standard library. > >> > > > >> > > On the other hand, the standard library likes to use exceptions in > >> case > >> > of > >> > > errors, and AFAIK we don't like exceptions in Impala. Also, we > already > >> > have > >> > > utility classes like ConditionVariable in the code base. In the > >> kudu/util > >> > > directory, there is a Mutex class which is more conform to the > Impala > >> > code > >> > > conventions than boost and the standard library. It also checks the > >> > > ownership of the mutex in debug mode, which can be useful for > >> detecting > >> > > bugs. > >> > > > >> > > Do you think it would be a good idea to create our own Mutex > >> > implementation > >> > > based on kudu/util/mutex? > >> > > > >> > > BR, > >> > > Zoltan > >> > > > >> > > >> > > > > > -- > Todd Lipcon > Software Engineer, Cloudera >
