Hi All, I just want to check if there is any feedback on this? Generated based on trunk version, this is to remove some code duplications/global variables. This also removes listener duplication when SO_REUSEPORT is not being used.
For details, please refer to Yann Ylavic's notes and my responses below. I also attached the code changes here again in case you missed it in the original email I sent last Friday. Thanks very much, 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.
httpd_trunk_so_reuseport_v2.patch
Description: httpd_trunk_so_reuseport_v2.patch
