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

[Owen Anderson]

On May 19, 2009, at 3:08 AM, Chris Lattner wrote:
> The basic algorithm looks fine, however:
>
> 1. Please move the meat of the code for LazyInit out of line somehow.
> Only the case when tmp != 0 should be inline ideally.

Done.

> 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.

> 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.

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.

> 4. Instead of going through the trouble to make the locking more
> 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.

--Owen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2620 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090519/f543b925/attachment.bin>