[kaffe] [gump] utf8const.c

[Guilhem Lavaux]

Helmer Krämer wrote:
> Guilhem Lavaux <guilhem at kaffe.org> wrote:
> 
> 
>>Helmer Krämer wrote:
>>
>>>Davanum Srinivas <dims at yahoo.com> wrote:
>>>
>>>
>>>
>>>>looks like we need a lock for locking access to utfLockRoot :(
>>>>
>>>
>>>
>>>I'd think that the whole utfLockRoot thing is broken (you'd
>>>need one utfLockRoot per thread since it's perfectly legal
>>>for two threads to work on utf8 constants at the same time).
>>>However, I think we could just remove the utfLockRoot thing
>>>completely, couldn't we? Even if one of the allocations in
>>>utf8const.c triggers a garbage collection, the thread holding
>>>the utf8Lock will be resumed before the finaliser starts to
>>>free unreachable objects (at least with kaffe-gc). This means
>>>that we don't have to drop the lock before doing an allocation,
>>>which renders utf8LockRoot useless. Or I just overlooked
>>>something ...
>>>
>>>Regards,
>>>Helmer 
>>
>>The problem is that utf constants are cached in a hash table. If two 
>>strings are exactly equals then utf8const does not create a new 
>>constant. I don't know if the approach is justified but this justifies 
>>the use UTF locking as only one thread at a time should touch the 
>>hashtable.
> 
> 
> Yeah, it's supposed to work pretty much like String.intern()
> works.
> 
> 
>>The assertion failure in itself is disturbing: it's like the 
>>fast lock subsystem is not working properly as it lets at least two 
>>threads running concurrently on that part of the code.
> 
> 
> UTFmalloc and UTFfree drop the utf8Lock before gc_malloc (or
> gc_free) and try to reacquire it afterwards. To do this, they
> use utfLockRoot to save the id of the thread holding the lock.
> So it looks like this is not working properly, but I'm also not
> sure whether we really need this:
> 
> Dropping the lock before invoking gc_malloc / gc_free only
> makes sense if a deadlock might occur if a thread holds the
> utf8Lock and is suspended in gc_malloc or gc_free. The gc
> thread itself however, will never try to acquire the utf8Lock,
> as it only scans the memory but neither allocates nor frees
> utf8 constants. The only gc-related thread that might try to
> acquire the utf8Lock is the finaliser thread, but that one can't
> cause a deadlock as the thread that invoked gc_malloc/gc_free
> will already have been resumed when the finaliser thread is
> running.
> 
> So I think we could use the normal lockStaticMutex/unlockStaticMutex
> functions to lock/unlock the utf8Lock and don't have to drop
> the utf8Lock during the calls to gc_malloc / gc_free (which
> means that we don't need the utfLockRoot variable any more).
> 

Hi,

Yes. Maybe this will improve performance. However our problem will still 
be there... There is another performance penalty I've seen in locks.c:
we are using KSEM(get) in getHeavyLock with a timeout. The timeout is 
expected to be the only signal which will make the call exit. This is 
unwise and we should have a mechanism to wake up one of the waiting 
thread. BTW, there is maybe something wrong here because a KSEM(put) can 
be caught by this KSEM(get) and not by the other one in slowLockMutex if 
the threads (3 at least) are wrongly synchronized.

Regards,

Guilhem.