Re: [RFT PATCH 2/2] win32: optimize pthread_cond_broadcast
- From
Paolo Bonzini <bonzini@gnu.org>
- Date
- Jun 8, 2010, 16:37 UTC
- Message-ID
- <4C0E71B2.1060904@gnu.org>
- In-Reply-To
- <4C0E7015.8030504@viscovery.net>
On 06/08/2010 06:30 PM, Johannes Sixt wrote:
Show 14 quoted lines
> Am 07.06.2010 15:38, schrieb Paolo Bonzini:
>> @@ -172,9 +172,10 @@ int pthread_cond_broadcast(pthread_cond_t *cond)
>> * As in pthread_cond_signal, access to cond->waiters and
>> * cond->was_broadcast is locked via the external mutex.
>> */
>> -
>> - if ((cond->was_broadcast = cond->waiters> 0)) {
>> + if (cond->waiters> 0) {
>> BOOLEAN result;
>> + cond->was_broadcast = cond->waiters> 1;
>> +
>
> It is possible that you set was_broadcast to 1 here, while another
> thread still sees was_broadcast == 0 in cond_wait.That still cannot happen, because pthread_cond_wait will be locked on the semaphore until the ReleaseSemaphore. The only race that exists is between broadcast/signal's ReleaseSemaphore and wait's WaitForSingleObject. This is benign, and exists before my patch. But in all cases the code before ReleaseSemaphore is serialized WRT to the code after wait's WaitForSingleObject.
> That said, as long as this series buys performance only at the expense > of clarity, I'm rather opposed to it because we do not call cond_wait > and cond_broadcast in time-critical paths.
Yes, it is less clear indeed. I tried to compensate with comments but that was not enough apparently. As I said I did this patch for another project where condvars are used in time-critical paths; if you do not want to keep it, that's not a problem.
Paolo