[LLVMdev] Multi-threading and mutexes in LLVM

[Chandler Carruth]

Zach, thanks for bringing the discussion to the full mailing list. While
some of this was hashed out in the review thread, I'll drop my personal
feelings here. Maybe will encourage others to chime in...

On Fri, Jun 6, 2014 at 6:47 PM, Zachary Turner <zturner at google.com> wrote:

> 1) Should support multi-threading be a compile-time or runtime parameter
> in LLVM?
>
> Currently it is both.  It is compile-time through the use of the
> define LLVM_ENABLE_THREADS, and it is runtime through the use of functions
> llvm_start_multithreaded, llvm_is_multithreaded, etc.  I and some others
> feel like runtime support for multi-threading could be removed, and it
> should be compile-time only.  However, I am not aware of all the ways in
> which this is being used, so this is where I would like some feedback.  The
> issues I have with runtime multithreading support are the following:
>
> * It leads to confusing code.  At any given point, is multi-threading
> enabled or disabled?  You never know without calling llvm_is_multithreaded,
> but even calling that is inherently racy, because someone else could
> disable it after it returns.
>
> * It leads to subtle bugs.  clang_createIndex, the first time it's called,
> enables multi-threading.  What happens if someone else disables it later?
>  Things like this shouldn't even be possible.
>
> * Not all platforms even support threading to begin with.  This works now
> because llvm_start_multithreaded(), if the compile time flag is set to
> disable threads, simply does nothing.  But this decision should be made by
> someone else.  Nobody really checks the return value from
> llvm_start_multithreaded anyway, so there's already probably bugs where
> someone tries to start multi-threading, and it fails.
>
> * What does it actually mean to turn multi-threading support on and off?
>
> Anybody that tries to do this is almost certainly broken due to some edge
> cases about when it's on and when it's off.  So this goes back to the first
> two points about confusing code and subtle bugs.
>

I share your concerns here. I think that it is a complete mistake to
support this at runtime.

Note, I'm not saying that we should penalize single threaded users of LLVM.
There are many legitimate uses of LLVM which reserve exactly one thread for
it and it seems reasonable for LLVM to try to make sure that use case
continues to work. I don't think removing the runtime configuration of this
would impact those users in any measurable way, or if it does I think those
are all bugs in how LLVM works and should be fixed irrespective of this
decision.


>
> 2) What should happen when you try to acquire a mutex in an app with
> threading disabled?
>
> If this is a compile-time parameter, the solution is simple: make an empty
> mutex class that satisfies the Lockable concept, and have its methods do
> nothing.  Then typedef something like llvm::mutex to be either std::mutex
> or llvm::null_mutex accordingly.  If this is a runtime parameter, it's more
> complicated, and we should ask ourselves whether or not it's necessary to
> avoid the overhead of acquiring an uncontended mutex in single threaded
> apps.  For what it's worth, all reasonable STL implementations will use a
> lightweight mutex implementation, which generally require less than 100
> nanoseconds to acquire uncontended, so I think we don't need to care, but
> again some feedback would be nice.
>

I would really like to move LLVM to a point where it never had a mutex lock
acquisition in such a critical path that removing it for single threaded
users was important. Any such use of a mutex is, IMO, a performance bug for
*both* the single threaded users and the multithreaded users! We shouldn't
hack around this by making a synchronization construct that isn't really a
synchronization construct. That makes LLVM increasingly hard to develop for
very marginal gains.

Note that I think your 100 nanosecond estimate is really conservative.
There are many mutex implementations even faster. I use the numbers also
cited by Jeff Dean and Peter Norvig which estimate it at 25 nanoseconds:
http://norvig.com/21-days.html#answers


> 3) We have some debug code in our mutex implementation that is intended to
> try to help catch deadlocks and/or race conditions.  Do we need this code?
>
> I think we can get by without it, but again some feedback about how, if at
> all, people are using this would be nice.  For example, if you want to
> detect deadlocks and race conditions you can use TSan.
>

I think that we should add such debug code to the std::mutex implementation
in libc++, and then most developers should have easy access to it. I'll
talk to Kostya and the folks that work on TSan to see if they would be up
for contributing it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140606/15b2026f/attachment.html>