[kaffe] Lock patch

[Helmer Krämer]

Guilhem Lavaux <guilhem at kaffe.org> wrote:

Hi,

> I've just finished a functional patch for the locking system. I don't 
> know yet how much slower it is. But I've already had some interesting 
> results with some private app which was not working previously. If noone 
> is against  I will commit it on wednesday...

I like the idea of cleaning up the locking subsystem. I also think that
it's a good idea to keep the heavy locks in place after they have been
allocated, since needing a heavy lock because of recursive locking or
contention means that this will happen again in the future.

If I got it right, the new code always allocates a heavy lock even if
there's no contention or recursive locking. Since I think that locking
an object only once by a single thread is likely to happen (think about
methods like ResourceBundle.getBundle() or BufferedInputStream.available()
and synchronized blocks in a method), I'm tempted to suggest a way to
avoid allocating a heavy lock in this case:

void locks_internal_lockMutex(iLock** lkp, iLock *heavyLock)
{
    if (!COMPARE_AND_EXCHANGE(lkp, LOCKFREE, KTHREAD(current)()))
        slowLockMutex(lkp);
}

void locks_internal_unlockMutex(iLock** lkp)
{
    if (!COMPARE_AND_EXCHANGE(lkp, KTHREAD(current)(), LOCKFREE))
        slowLockMutex(lkp);
}

If nobody owns the lock, the jthread_t of the current thread is stored in
*lkp (afterwards called thin lock). If the same thread tries to lock the
mutex again, or another thread tries to do so, a heavy lock will be allocated.
Furthermore, getHeavyLock has to be modified as well:

#define IS_HEAVY_LOCK(ptr) (((ptr)&1)==1)

static iLock *getHeavyLock(iLock* volatile *lkp)
{
    iLock* lk;

    for (;;)
    {
        lk = *lkp;

        if (!IS_HEAVY_LOCK(lk))
        {
           iLock* new = gc_malloc (sizeof(iLock), KGC_ALLOC_LOCK);

	   // initialize new
	   ...
           lk->in_progress = 0;

           // preserve current state of lock
	   new->holder = lk;
           if (lk != LOCKFREE)
              new->lockCount = 1;

           if (!COMPARE_AND_EXCHANGE(lkp, lk, new|1))
           {
               gc_free (lk);
               continue;
           }
        }

        // clear lowest bit so lk is the real pointer
        lk &= ~1;

        for (;;)
        {
            // wait until lk->in_progress is 0
        }

        return lk;
    }
}

If *lkp does not point to a heavy lock, a new one is allocated on the heap.
To preserve the state of the lock when a thin lock is replaced by a heavy
lock, new->holder is initialized with the current value of lk, and lockCount
is set if necessary. Afterwards an attempt is made to install the heavy lock.
If that doesn't work (either because the lock was freed or because another
thread already installed a heavy lock), the heavy lock is freed and we start
again with checking whether *lkp is a heavy lock. Once we got a heavy lock,
we wait for the heavy lock to become usable by the current thread, just like
your code does. 

What do you think of this?

Regards,
Helmer