* Re: Dead code in pthread_cond_wait() for spin-wait [not found] <87a5spvwoi.fsf@laura> @ 2023-10-11 18:36 ` Mathieu Desnoyers [not found] ` <d38585f6-d896-47bc-8716-1a0a671f7f45@linaro.org> 1 sibling, 0 replies; 3+ messages in thread From: Mathieu Desnoyers @ 2023-10-11 18:36 UTC (permalink / raw) To: Olivier Dion; +Cc: libc-help, libc-alpha, carlos, Florian Weimer On 2023-10-11 14:30, Olivier Dion wrote: > Hi, > > Commit ed19993b5b0d05d62cc883571519a67dae481a14 from 2016 introduce "a > new implementation of condition variables". This new implementation > seems to introduce a spin-wait fast-path to avoid taking the underlying > futex when doing a pthread_cond_wait(). Adding libc-alpha, Carlos and Florian in CC. Thanks, Mathieu > > However, looking at the following snippet in nptl/pthread_cond_wait.c: > > static __always_inline int > __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex, > clockid_t clockid, const struct __timespec64 *abstime) > { > const int maxspin = 0; > [...] > /* Spin-wait first. [...] */ > unsigned int spin = maxspin; > while (signals == 0 && spin > 0) { > [...] > } > [...] > } > > it seems to me that the try-spinning portion is dead code in > pthread_cond_wait_common(). I was wondering why this was the case? > > I also think that there would maybe some interest in using the Userspace > RCU wait algorithm [0] for this. > > [0] https://github.com/urcu/userspace-rcu/blob/master/src/urcu-wait.h > > Thanks, > > Olivier > -- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <d38585f6-d896-47bc-8716-1a0a671f7f45@linaro.org>]
* Re: Dead code in pthread_cond_wait() for spin-wait [not found] ` <d38585f6-d896-47bc-8716-1a0a671f7f45@linaro.org> @ 2023-10-12 15:45 ` Olivier Dion 2023-10-12 16:40 ` Florian Weimer 0 siblings, 1 reply; 3+ messages in thread From: Olivier Dion @ 2023-10-12 15:45 UTC (permalink / raw) To: libc-help, libc-alpha Cc: Mathieu Desnoyers, Adhemerval Zanella Netto, carlos, Florian Weimer On Wed, 11 Oct 2023, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote: > On 11/10/23 15:30, Olivier Dion via Libc-help wrote: [...] >> However, looking at the following snippet in nptl/pthread_cond_wait.c: >> >> static __always_inline int >> __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex, >> clockid_t clockid, const struct __timespec64 *abstime) >> { >> const int maxspin = 0; >> [...] >> /* Spin-wait first. [...] */ >> unsigned int spin = maxspin; >> while (signals == 0 && spin > 0) { >> [...] >> } >> [...] >> } >> > > Afaik it was added a placeholder for future extension, so fell free to > send a patch to delete it. [...] What about making it work instead? Would this be something to consider? IMHO it would be worth it. Having a fast-path that spins instead of taking the futex directly can greatly impact performance in some cases. In the meantime, I think it is safe to remove it. I'll send a patch on libc-alpha for that. Thanks, Olivier -- Olivier Dion EfficiOS Inc. https://www.efficios.com ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Dead code in pthread_cond_wait() for spin-wait 2023-10-12 15:45 ` Olivier Dion @ 2023-10-12 16:40 ` Florian Weimer 0 siblings, 0 replies; 3+ messages in thread From: Florian Weimer @ 2023-10-12 16:40 UTC (permalink / raw) To: Olivier Dion Cc: libc-help, libc-alpha, Mathieu Desnoyers, Adhemerval Zanella Netto, carlos * Olivier Dion: > On Wed, 11 Oct 2023, Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> wrote: >> On 11/10/23 15:30, Olivier Dion via Libc-help wrote: > > [...] > >>> However, looking at the following snippet in nptl/pthread_cond_wait.c: >>> >>> static __always_inline int >>> __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex, >>> clockid_t clockid, const struct __timespec64 *abstime) >>> { >>> const int maxspin = 0; >>> [...] >>> /* Spin-wait first. [...] */ >>> unsigned int spin = maxspin; >>> while (signals == 0 && spin > 0) { >>> [...] >>> } >>> [...] >>> } >>> >> >> Afaik it was added a placeholder for future extension, so fell free to >> send a patch to delete it. > > [...] > > What about making it work instead? Would this be something to consider? > IMHO it would be worth it. Having a fast-path that spins instead of > taking the futex directly can greatly impact performance in some cases. Changes to the condvar code are very difficult for us. There's an open bug with patches that have been under review for years: pthread_cond_signal failed to wake up pthread_cond_wait due to a bug in undoing stealing <https://sourceware.org/bugzilla/show_bug.cgi?id=25847> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-10-12 16:40 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <87a5spvwoi.fsf@laura> 2023-10-11 18:36 ` Dead code in pthread_cond_wait() for spin-wait Mathieu Desnoyers [not found] ` <d38585f6-d896-47bc-8716-1a0a671f7f45@linaro.org> 2023-10-12 15:45 ` Olivier Dion 2023-10-12 16:40 ` Florian Weimer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).