+1 Interesting. If this proves to work reliably for mpm_event, it could be reused in mod_http2 as a stepping stone for further mpm integration. ATM mod_http2 uses a conditional for wait/signalling with short timeouts and read attempts on its main connection. If it used a wakeup pipe instead of the conditional, no timeout stuttering would be needed.
> Am 21.08.2016 um 04:14 schrieb Yann Ylavic <ylavic....@gmail.com>: > > On Thu, Aug 18, 2016 at 3:17 PM, Yann Ylavic <ylavic....@gmail.com> wrote: >> >> I was thinking of something like the attached patch, where we compute >> the exact/minimal timeout needed for the queues. >> >> It requires walking/locking the queues (looking at the first item >> only), but besides getting the exact poll() timeout which helps >> spurious wakeups, it also allows to avoid walking this queues after >> the poll() (previously each 0.1s, for maintenance) when not necessary. >> So overall I think it's a gain. > > Finally I came up with another way which requires no walking (hence > locking) in the listener outside queues/timers processing (patch > attached). > It looks good to me, not heavily tested/debugged still... > > The approach is, for both timeout queues and timers, to maintain the > next (lowest) expiry time of all entries (namely queues_next_expiry > and timers_next_expiry). > These global values are kept up to date in TO_QUEUE_APPEND (for > timeout queues) and event_get_timer_event (for timers, where the > skiplist is filled) by comparing the expiry of newly added entries > (already protected by their respective timeout/skiplist mutex). > This happens in the workers, and is cheap (locking was there already > to protect from concurrent workers), it just requires an > apr_pollset_wakeup to notice the listener whenever one of these > expiries is updated. > > So now, in the listener, we can read these values directly to compute > the min poll() timeout to use, and also for the timeout queues to > determine whether or not it's time to process them. > If an expiry is updated while poll()ing, it's woken up and the timeout > is also updated. > No locking needed here, I use volatile reads which is enough (no > atomic/ordered read required, no apr_atomic_read64 to handle an > apr_time_t anyway) since the listener is garanteed to be woken up when > needed. > > This also benefits the !listener_is_wakeable case (with a maximum > poll() timeout of 100ms), since we avoid the lock when no timer is > armed (we don't use timers in 2.4.x afaict), and also avoid spurious > timeout queues processing. > > The patch includes Luca's http://apaste.info/mke (base of > APR_POLLSET_WAKEABLE handling) and also tries to make existing > critical sections as short as possible (barely related, but while at > it :) > > Thoughts? > > > Regards, > Yann. > <mpm_event-wakeup.patch>
