On Wed, 13 Oct 2010 15:40:22 +0100, Peter Payne <sourcefo...@pirosa.co.uk> wrote: > Hello Amos, > > apologies to the dev list for what must appear to be spamming. >
Completely on topic. No apologies needed. > Please use the attached version of the file. It has a minor performance > tweak (only set events that have changed if not clearing any events). > > Compiled using CC=/tool/sunstudio12/bin/cc and tested again using Apache > Bench and some manual queries through a web browser. > > Peter > Will do. Thanks. > On 10/13/10 11:21, Peter Payne wrote: >> Hello Amos, >> >> in response to your comments. >> >> >> BENCHMARKS >> >> Firstly, I wanted to address the benchmark questions as it made me >> curious as to whether there really was an advantage in using >> /dev/poll. I used the Apache Bench tool (that comes with HTTPD) to do >> my benchmarks. >> >> I compiled a 32-bit Solaris 5.10 x86 of Squid version 3.1.8, and >> another Squid version 3.1.8 with my patches. I shall call them >> "unpatched Squid" and "patched Squid" respectively. Note that >> "unpatched Squid" was using ordinary poll() and "patched Squid" was >> using /dev/poll. >> >> Where I used Apache bench with 1 simultaneous connection or even 8,000 >> simultaneous connections there was little difference between >> "unpatched Squid" and "patched Squid". When all connections are busy >> there is no performance gain. >> >> However the test that proved the most striking difference was >> pre-establishing 8,000 TCP socket connections to Squid using a basic >> Perl script. Then running Apache Bench with 1 connection making >> 100,000 keep-alive'd requests. >> >> unpatched Squid (using poll): >> 2min23sec CPU user-time >> 171sec to process 100000 Apache Bench queries >> >> patched Squid (using /dev/poll): >> 0min22sec CPU user-time >> 25sec to process 100000 Apache Bench queries >> Oooh... Speed. >> Clearly /dev/poll has a significant performance gain where there are >> many idle TCP socket connections. >> >> I have now also optimised the comm_devpoll.cc routine to ensure >> unnecessary calls to /dev/poll with the POLLREMOVE flag were removed. >> >> >> CODE AUDIT >> >> Most of the style used was a result of directly copying the styles >> used in the existing comm_devpoll.c from Squid v2.7 and comm_epoll.cc >> from Squid v3.1.8. However where these conflict with the requests made >> most recently I will adopt the recommendations in the e-mail. >> >> 1. used #if USE_DEVPOLL instead of #ifdef >> 2. moved #includes except squid.h inside USE_DEVPOLL wrapper >> 3. moved comment to own line >> 4. removed big XXX..XXX lines (note comm_epoll.cc uses this notation) >> 5. made big doxygen effort!!! Enjoy. >> 6. (did not remove no-op else cases, I think they are clear) >> 7. used HERE macro as requested >> >> >> SQUID-COMPAT.DIFF >> >> /dev/poll doesn't need to be hard coded, that was a mistake. Please >> migrate USE_DEVPOLL conditional to same line as USE_EPOLL. >> K. will do >> >> SQUID-ROOT.DIFF >> >> Pretty much all beyond me, am happy with whatever changes you decide >> to make. I just wanted to add a /dev/poll auto-detect routine and use >> that in preference to poll() if available. >> >> Note that epoll/kqueue is assumed to be unavailable on any Solaris >> system with /dev/poll. You might want to thus prioritise /dev/poll >> support to BELOW that of epoll and kqueue (in case there's any >> confusion). But of course /dev/poll should be by default selected if >> available before defaulting to standard poll(). Okay. Thanks for that. >> >> Kind regards, >> Peter Payne >> +1 from me with merge tweaks. Unless anyone has objections I will commit with tweaks at the next opportunity. Amos