> From: Brian Pane [mailto:[EMAIL PROTECTED] > > [EMAIL PROTECTED] wrote: > > >As promised, a new implementation of apr_poll, which should improve > >performance. I have only implemented the HAVE_POLL case so far, but the > >API is 99% the same, so I have no fear that the others are not > >possible. In fact, the ONLY change to the function API, is that > >apr_poll() itself takes one more argument. The types however are now > >complete, and apr_poll can be used on both files and sockets (as well as > >anything else we want to add). > > > > I really like the basic design concept for this. It provides a > good abstraction that would allow us to plug in /dev/poll in the > future if needed. > > A couple of suggestions on the details: > > * Instead of having apr_poll_setup just create an array of poll > objects, it may be more flexible to have it create a wrapper object > that contains the array. (Just in case we need to add anything else > to it in the future, like a file handle for /dev/poll.)
I considered a wrapper object that contains the array, but that is what we started with. Having a wrapper object means that the user either needs to manage both the wrapper and the actual object, or we need to have a set of functions to create the inner object. Either one seemed more complex than I wanted it to be. Since the whole point of poll is to provide ultimate performance, I wanted to give the user control over the memory as much as possible. As for /dev/poll support, the point of apr_poll() in this design, is that function converts from the apr_pollfd_t to the interior functions. I don't have a great answer for how to /dev/poll, but my initial answer would be to add a void pointer to the apr_pollfd_t for different apr_poll() implementations to store arbitrary data. > * The apr_poll_socket_add function looks like it's O(n). That could be > a problem if we ever need to poll a large number of descriptors (like > in an event-loop server). Adding a faster lookup would make the data > structure more complicated, though. Maybe it's best to just stick > with the O(n) array approach for now, but encapsulate the data > structures > so that we can swap in a new design later if needed? It is O(n), but it should never be used. The fastest way to add sockets or files to the pollset is to add them directly. We can and should make it easier to do this by creating a couple of macros. Both of those functions have been marked as deprecated. The intent is that people will create their apr_pollfd_t array themselves and they will own maintaining it. I have only left those functions around for backwards compat, because I didn't feel like fixing all of Apache today. Thanks for the feedback. Ryan
