Hi Yingqi, commited in r1635521, with some changes (please see commit log). These are not big changes, and your work on removing the global variables and restoring some previous behaviour is great, thanks for the good work.
Regards, Yann. On Wed, Oct 29, 2014 at 6:36 PM, Lu, Yingqi <yingqi...@intel.com> wrote: > Thank you very much for your help! > > Thanks, > Yingqi > > -----Original Message----- > From: Yann Ylavic [mailto:ylavic....@gmail.com] > Sent: Wednesday, October 29, 2014 10:34 AM > To: httpd > Subject: Re: Listeners buckets and duplication w/ and w/o SO_REUSEPORT on > trunk > > Hi Yingqi, > > I'm working on it currently, will commit soon. > > Regards, > Yann. > > On Wed, Oct 29, 2014 at 6:20 PM, Lu, Yingqi <yingqi...@intel.com> wrote: >> Hi All, >> >> I just want to check if there is any feedback/comments on this? >> >> For details, please refer to Yann Ylavic's notes and my responses below. >> >> Thanks, >> Yingqi >> >> -----Original Message----- >> From: Lu, Yingqi [mailto:yingqi...@intel.com] >> Sent: Friday, October 10, 2014 4:56 PM >> To: dev@httpd.apache.org >> Subject: RE: Listeners buckets and duplication w/ and w/o SO_REUSEPORT >> on trunk >> >> Dear All, >> >> Attached patch is generated based on current trunk. It covers for >> prefork/worker/event/eventopt MPM. It supposes to address following issues >> regarding to SO_RESUEPORT support vs. current trunk version: >> >> 1. Same as current trunk version implementation, when active_CPU_num <= 8 or >> when so_reuseport is not supported by the kernel, ap_num_buckets is set to >> 1. In any case, there is 1 dedicated listener per bucket. >> >> 2. Remove global variables (mpm_listen, enable_default_listeners and >> num_buckets). mpm_listen is changed to MPM local. enabled_default_listener >> is completely removed. num_buckets is changed to MPM local (ap_num_buckets). >> I rename have_so_reuseport to ap_have_so_reuseport. The reason for keeping >> that global is because this variable is being used by ap_log_common(). Based >> on the feedback here, I think it may not be a good idea to change the >> function interface. >> >> 3. Change ap_duplicated_listener to have more parameters. This function is >> being called from MPM local (prefork.c/worker.c/event.c/eventopt.c). In this >> function, prefork_listener (or worker_listener/even_listener/etc) array will >> be initialized and be set value. ap_num_buckets is also calculated inside >> this function. In addition, this version solves the issue with "one_process" >> case (current trunk version has issue with one_process enabled). >> >> 4. Change ap_close_listener() back to previous (2.4.X version). >> >> 5. Change dummy_connection back to previous (2.4.X version). >> >> 6. Add ap_close_duplicated_listeners(). This is called from mpms when >> stopping httpd. >> >> 7. Add ap_close_child_listener(). When listener_thread (child process in >> prefork) exit, only the dedicated listener needs to be closed (the rest are >> already being closed in child_main when the child process starts). >> >> 8. Remove duplication of listener when ap_num_buckets = 1 or without >> SO_REUSEPORT support (ap_num_buckets = 1). With so_reuseport, only >> duplicated (ap_num_buckets - 1) listeners (1 less duplication less current >> trunk implementation). >> >> 9. Inside each mpm, move child_bucket, child_pod and child_mutex >> (worker/prefork only) to a struct. Also, add member bucket to the same >> struct. >> >> Please review and let me know your feedback. >> >> Thanks, >> Yingqi >> >> -----Original Message----- >> From: Yann Ylavic [mailto:ylavic....@gmail.com] >> Sent: Tuesday, October 07, 2014 5:26 PM >> To: httpd >> Subject: Re: Listeners buckets and duplication w/ and w/o SO_REUSEPORT >> on trunk >> >> On Wed, Oct 8, 2014 at 2:03 AM, Yann Ylavic <ylavic....@gmail.com> wrote: >>> On Wed, Oct 8, 2014 at 1:50 AM, Lu, Yingqi <yingqi...@intel.com> wrote: >>>> 3. Yes, I did use some extern variables. I can change the name of them to >>>> better coordinate with the variable naming conversion. We should do >>>> something with ap_prefixed? Is there anything else I should consider when >>>> I rename the variable? >>> >>> Maybe defining new functions with more arguments (to be used by the >>> existing ones with NULL or default values) is a better alternative. >> >> For example, ap_duplicate_listeners could be modified to provide mpm_listen >> and even do the computation of num_buckets and provide it (this is not an >> API change since it is trunk only for now). >> >> ap_close_listeners() could be then restored as before (work on ap_listeners >> only) and ap_close_duplicated_listeners(mpm_listen) be introduced and used >> in the MPMs instead. >> >> Hence ap_listen_rec *mpm_listeners could be MPM local, which would >> then call ap_duplicate_listeners(..., &mpm_listeners, &num_buckets) >> and ap_close_duplicated_listeners(mpm_listeners) >> >> That's just a quick thought... >> >>> >>> Please be aware that existing AP_DECLAREd functions API must not change >>> though. >>> >>> Regards, >>> Yann. >>> >>>> >>>> Thanks, >>>> Yingqi >>>> >>>> >>>> -----Original Message----- >>>> From: Yann Ylavic [mailto:ylavic....@gmail.com] >>>> Sent: Tuesday, October 07, 2014 4:19 PM >>>> To: httpd >>>> Subject: Listeners buckets and duplication w/ and w/o SO_REUSEPORT >>>> on trunk >>>> >>>> Hi, >>>> >>>> some notes about the current implementation of this (trunk only). >>>> >>>> First, whether or not SO_REUSEPORT is available, we do duplicate the >>>> listeners. >>>> This, I think, is not the intention of Yingqi Lu's original proposal, and >>>> probably my fault since I asked for the patch to be splitted in two for a >>>> better understanding (finally the SO_REUSEPORT patch only has been >>>> commited). >>>> The fact is that without SO_REUSEPORT, this serves nothing, and we'd >>>> better use one listener per bucket (as originally proposed), or a single >>>> bucket with no duplication (as before) if the performance gains do not >>>> worth it. >>>> WDYT? >>>> >>>> Also, there is no opt-in/out for this functionalities, nor a way to >>>> configure number of buckets ratio wrt number of CPUs (cores). >>>> For example, SO_REUSEPORT also exists on *BSD, but I doubt it would work >>>> as expected since IFAICT this not the same thing as in Linux (DragonFly's >>>> implementation seems to be closed to Linux' one though). >>>> Yet, the dynamic setsockopt() check will also succeed on BSD, and the >>>> functionality be enabled. >>>> So opt in (my preference) or out? >>>> >>>> Finally, some global variables (not even ap_ prefixed) are used to >>>> communicate between listen.c and the MPM. This helps not breaking the API, >>>> but this is trunk... >>>> I guess we can fix it, this is just a (self or anyone's) reminder :) >>>> >>>> Regards, >>>> Yann.
