RE: Synchronization article on developerWorks

["Boehm, Hans" <hans_boehm at hp dot com>]

> From: Jeff Sturm [mailto:jsturm@one-point.com]
> Last I checked, linuxthreads didn't spin on a mutex... after one
> test_and_set failure it falls into a nanosleep.  That may 
> have changed in
> newer releases.
I believe __pthread_lock now does, at least if __pthread_smp_kernel is set.
(Hopefully that doesn't get set on uniprocessors that happen to have an smp
kernel installed?)  Pthread_mutex_lock usually invokes that.
> 
> I'm curious if the path for reentrant monitors couldn't be shortened
> somewhat.  In _Jv_MonitorExit we have
> 
>   if (__builtin_expect(light_thr_id == self, true))
>     {
>       count = he -> light_count;
>       if (__builtin_expect((address & ~HEAVY) == addr, true))
>         {
>           if (count != 0)
>             {
>               // We held the lightweight lock all along.  
> Thus the values
>               // we saw for light_thr_id and light_count must 
> have been
>               // valid.
>               he -> light_count = count - 1;
>               return;
> 
> No compare_and_swap here.  I guess if a thread evaluates 
> "light_thr_id ==
> self" true it knows it holds the lock.  Why can't 
> _Jv_MonitorEnter do the
> same and potentially avoid the CAS?  Am I missing something?
I don't think that patch is sufficient, since you also need to check that
the address corresponds to the object you're looking for.  Even if it is,
you would neeed to increment the light_count field, unlike the other case.
But all of that's probably fixable.

The harder question is whether the added test is worth it.  I believe the
rule of thumb in one of the IBM papers was that reentrant locks have about
10% of the frequency of initial uncontested locks.  At that rate it's
probably not worth it.  But I gather those statistics vary for other
applications.

Hans
> 
> Index: natObject.cc
> ===================================================================
> RCS file: /cvs/gcc/gcc/libjava/java/lang/natObject.cc,v
> retrieving revision 1.17
> diff -u -r1.17 natObject.cc
> --- natObject.cc        2001/07/23 03:51:17     1.17
> +++ natObject.cc        2001/07/25 04:46:49
> @@ -768,8 +769,8 @@
> 
>    assert(!(addr & FLAGS));
>  retry:
> -  if (__builtin_expect(compare_and_swap(&(he -> address),
> -                                       0, addr),true))
> +  if (__builtin_expect (he->light_thr_id != self
> +      && compare_and_swap(&(he -> address), 0, addr), true))
>      {
>        assert(he -> light_thr_id == INVALID_THREAD_ID);
>        assert(he -> light_count == 0);
> 
>