[llvm-commits] [PATCH] Thread-safe ManagedStatic

[Chris Lattner]

>> 2. Again, we only need a single global lock, we do not need a mutex
>> per ManagedStatic.
>
> Conversely, why have only one?  Also, see my answer to #3.

Because it takes less space.

>> 3. Why do you create your own mutex out of atomic operations instead
>> of just using a pthreads lock?  spinning is not always an efficient
>> solution to contention, particularly if your machine really only has
>> one cpu! (in which case you end up spinning away the rest of your
>> timeslice)
>
> Portability.  llvm::sys::Mutex _cannot_ be made to work, because on  
> Windows
> it has to have a non-trivial constructor.

This is irrelevant.  We are going to have a bunch of "global" state  
for all the stuff that uniques stuff.  The managed static is just the  
first piece of it.  When the app calls llvm_start_multithread_mode(),  
this function can set up the global mutex and do whatever else it  
wants to do, while the app is still single threaded.  This means that  
it is perfectly safe to have:

Mutex *TheGlobalMutex;

and use:

TheGlobalMutex->lock();

places, because you know that TheGlobalMutex will be allocated and  
initialized when the app is single threaded.


> Also, this is pretty much the ideal case for spinlocks.   
> Initialization only happens once and
> we're using a separate mutex for each object, so we don't expect  
> significant contention over
> the life of the program.   Plus, the initialization itself should be  
> fairly fast.  This means that
> the spinning thread can continue immediately after the lock is  
> released, rather than having
> to wait for the OS to wake it up.

ManagedStatic is a generic class, meaning that it can take an  
arbitrary amount of time to initialize.  One initialization can  
potentially also cause initialization of other objects: make sure the  
global mutex is recursive.

>>
>> efficient, I think it would be good to special case the scenario when
>> there is only one LLVM thread.  Why not make clients explicitly opt- 
>> in
>> to multithreaded llvm, by making an explicit llvm_multithread call?
>> If multithreading is disabled, this call should return an error.  If
>> enabled, it would set a global.  All the various "locking" clients
>> could just check the global before taking potential heavy-weight  
>> locks.
>
> We already handle this in a somewhat different way.  When threading  
> is disabled,
> llvm::sys::Mutex operations are turned into no-ops, and  
> MemoryFence() and
> CompareAndSwap() are given non-atomic implementations.

You are confusing the case when llvm is compiled without  
multithreading support with the case where it is compiled with  
threading support but that app is not using it.

-Chris