[M3commit] CVS Update: cm3

[Jay]

This isn't one-time initialization, it is every condition variable or mutex initialization. Doing a heap allocation with more locks held than necessary seems not good -- hold locks for shorter time if possible.
 
 
Granted, this does result in more locks/unlocks.
It /could/ be that the underlying heap has some optimizations that cause it to scale nicely, defeated by us holding a lock while calling into it.
Or it could be that the underlying heap is guarded by exactly the same sort of lock, in which case my change doesn't help.
 
 
I do believe it is safe.
I still check and commit the value under the lock, I just moved the bulk of the work to outside the lock -- with a "risk" that the bulk of the work will be performed more than once but that is generally ok.
 
 
But either way is ok with me.
This would be a good place to use "CAS".
 
 
 - Jay



________________________________
> From: hosking at cs.purdue.edu
> To: jkrell at elego.de
> Date: Tue, 5 May 2009 09:07:51 +1000
> CC: m3commit at elegosoft.com
> Subject: Re: [M3commit] CVS Update: cm3
>
> Is this safe? Double-check pattern is often not fully safe. I haven't looked at your code but will do so ASAP.
>
> PS Why are you optimizing initialization -- seems premature to me.
>
> Antony Hosking | Associate Professor | Computer Science | Purdue University
> 305 N. University Street | West Lafayette | IN 47907 | USA
> Office +1 765 494 6001 | Mobile +1 765 427 5484
>
>
>
> On 4 May 2009, at 13:19, Jay Krell wrote:
>
> CVSROOT: /usr/cvs
> Changes by: jkrell at birch. 09/05/04 13:19:07
>
> Modified files:
> cm3/m3-libs/m3core/src/thread/PTHREAD/: ThreadPThread.m3
>
> Log message:
> move expensive calloc+pthread_mutex_init outside lock, using a double
> check pattern:
>
> if it appears work is needed (but with taking lock to be sure)
> do the work, into temp
> take lock
> if work not actually needed, ok
> release lock
> free the temp (don't care if it succeeded)
> else
> if attempt to do the work failed
> release lock
> raise exception
> else
> commit the work
> release lock
> end
> end
> end
>
> It is ugly perhaps to repeat "release lock" three times instead
> of once, but time holding the lock is minimized.
>
> As long as "temp" is a pointer, which it is here,
> this can use InterlockedCompareExchangePointer and not a full blown lock.
>
> Something like:
>
> if pointer == null
> temp = new ...
> if InterlockedCompareExchange(&pointer, NULL, temp) != NULL)
> delete temp (* lost the race, ok *)
> else
> if temp == NULL (* won the race but failed *)
> raise exception
> end
> end
> end
>
> This pattern is bad if the temp work is particularly expensive or
> must not be executed more than once. That leaves
> many instances in which it is ok.
>
> Also here eliminate PushFrame, but the point is more to eliminate
> calloc from inside a lock. Therefore, if calloc is well tuned for
> multiprocessor systems, and many locks are used for the first
> time at about the same time on multiple threads, scale better.
> ("hold locks for minimal time, even at the risk of doing more work
> in the event of a race" is a good scalability principle; as well as "replace lock
> with interlocked/atomic/cas/etc., also at the risk of doing
> more work when there is a race")
>
> Factor InitMutex and InitCondition into one function InitMutexHelper.
> It is tempting to rewrite InitMutexHelper in C, thereby removing
> the exposure of the init lock, however as I understand this is a
> bad move due to interior gc pointers or such.
>