djm at mindrot.org
Sat Aug 4 11:22:48 EST 2007
On Fri, 3 Aug 2007, Chris Rapier wrote:
> I've updated the channel patch including some suggestions. The main
> difference is that I eliminated the channels array entirely (and
> the attendant code to create the array). I did not, however, include
> linked list pointers in the Channel struct. Mostly because its easier
> for me to work with at this time. I expect I'll do it in the next
> iteration though.
> Removing the channels array does mean that channel_by_id must iterate
> through a linked list rather than doing an index lookup. I don't think
> this will have much impact on performance in the majority of cases. On
> the plus side, I think I can eliminate a lot of the NULL conditionals.
> I haven't yet though.
> Anyway, if anyone is interested the patches are attached. Any
> comments, observations, or such not is greatly appreciated.
> At this time this should still be viewed as development code.
> Preliminary tests indicated that there were no performance hits and
> there were some hints that in some situations it will provide a
> performance improvement.
Before you go any further with this diff, please understand that it
almost no chance of being incorporated unless it follows the
style of the existing code.
We also prefer to use the queue macros instead of hand-rolled
linked list implementations, though I'm not sure that a linked list is
the right data structure (remember you need to support by-id lookups
too), whether you need to change the data structure at all or whether
traversing an array of a few dozen elements is worth optimising.
It seems you could avoid most of the channel array scans by simply
tracking the number of channels actually used and stopping the loop
after seeing that many open channels (this would work very well unless
the entries were fragmented across the array). If you wanted to be
tricky, you could add a precomputed skip step to the channels array that
would point to the next allocated channel, allowing the loop to skip
past unused channels.
More information about the openssh-unix-dev