[quagga-dev 10197] Re: [PATCH 2/4] hash: force size to be a power of 2

Certain, Andrew certain at amazon.com
Tue Jan 15 18:58:55 GMT 2013



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> Sent: Tuesday, January 15, 2013 9:47 AM
> To: David Lamparter
> Cc: Stephen Hemminger; quagga-dev at lists.quagga.net
> Subject: [quagga-dev 10194] Re: [PATCH 2/4] hash: force size to be a power
> of 2
> 
> On Tue, 15 Jan 2013 17:49:10 +0100
> David Lamparter <equinox at opensourcerouting.org> wrote:
> 
> > On Fri, Jan 04, 2013 at 03:29:21PM -0800, Stephen Hemminger wrote:
> > > By forcing the hash table size to be a power of 2, a potentially
> > > expensive divide can be replaced by a mask operation. Almost all
> > > usage of the hash table was using default size of 1024. Only places
> > > with different size was thread library (1011) and bgp aspath.
> >
> > Since I can't really estimate the impact this has (in particular the
> > AS-Path part?), I'm going to defer this (and the other hash patch from
> > your next series) until after 0.99.22.
> >
> > Looks good otherwise,
> >
> > -David
> 
> As best I can tell the reason the original developer wanted to use
> 1011 rather than 1024 was that  the thread library is using the function
> address is used as hash key, and the function address will be aligned.
> 
> I didn't see enough threads to worry about it, and the hash lookup of
> thread is only done when thread is created.
>
Looks like our emails crossed in the ether....

It seems that you're trading one performance improvement against another.  The hashing performance of the thread look up will be worse, and you're arguing that this performance hit isn't as important as removing the divide.  You may be right, but I would like to see the data.

Obviously, my proposal would be just as bad as yours in impacting the hashing of the threads.  Without a coherent and comprehensive test suite, no changes can be made with high confidence, which in my mind raises the bar for when we should accept change.  Until such a thing exists, I think that any changes done solely for performance reasons should only be accepted if there is a clear problem with the current code, rather than just for general cleanup reasons (I'm not saying you don't have a quantitative argument; I'm just asking you to share it).

Andrew




More information about the Quagga-dev mailing list