William A. Rowe, Jr. wrote:
At 11:21 PM 7/18/2002, Brian Pane wrote:
I'm almost in complete agreement with this proposal. We need a wrapper object to hold the temp pollset so that we can re-use it for all subsequent poll calls on the same fd set.
Notice that apr_pollfd_internal_t *int; below _IS_ entirely
opaque... [and using a reserved keyword, the member name
REALLY sucked... how about simply *private.]
Right, but the pointer to the vector of apr_pollfd_t objects is transparent. And that's the one that will cause problems if we try to use something like /dev/poll, because the set of descriptors in that vector can get out of synch with the descriptors that have been registered with /dev/poll.
(Yes, we could avoid that problem by including within the apr_pollfd_internal_t a list of the descriptors that we used on the last poll call for this apr_pollfd_set_t, which would make it possible to detect added/removed descriptors since the last call. But that would require an O(n)-time scan through the entire vector on each request, which defeats the scalability advantages of using a faster poll interface.)
Speaking of linear-time code, I just checked the current apr_poll(), and it's doing an O(n)-time loop to copy from the apr_pollfd_t structs to the pollfd structs. That's a major scalability problem for larger values of n.
Fortunately, your new design offers a way to fix the O(n) problem: build the pollfd vector once, and re-use it on subsequent calls if the apr_pollfd_t vector hasn't changed. (The transparency of the apr_pollfd_t vector and apr_pollfd_set_t makes this a little tricky, though.)
--Brian
