code review 7400055: net: fix slow network interface manipulations

net: fix slow network interface manipulations

This CL reduces unnecessary network facility lookups introduced
by recent changes below.

changeset: 15798:53a4da6a4f4a
net: return correct point-to-point interface address on linux

changeset: 15799:a81ef8e0cc05
net: set up IPv6 scoped addressing zone for network facilities

Also adds a test case for issue	4839.

Benchmark results on linux/amd64, virtual machine:
benchmark                                 old ns/op    new ns/op    delta
BenchmarkInterfaces-2                         80487        80382   -0.13%
BenchmarkInterfaceByIndex-2                   72013        71391   -0.86%
BenchmarkInterfaceByName-2                    79865        80101   +0.30%
BenchmarkInterfaceAddrs-2                     42071       829677  +1872.09%
BenchmarkInterfacesAndAddrs-2                 35016       607622  +1635.27%
BenchmarkInterfacesAndMulticastAddrs-2       169849       169082   -0.45%
old: 15797:9c3930413c1b, new: tip

Benchmark results on linux/amd64, virtual machine:
benchmark                                 old ns/op    new ns/op    delta
BenchmarkInterfaces-2                         80487        81459   +1.21%
BenchmarkInterfaceByIndex-2                   72013        71512   -0.70%
BenchmarkInterfaceByName-2                    79865        80567   +0.88%
BenchmarkInterfaceAddrs-2                     42071       120108  +185.49%
BenchmarkInterfacesAndAddrs-2                 35016        33259   -5.02%
BenchmarkInterfacesAndMulticastAddrs-2       169849        82391  -51.49%
old: 15797:9c3930413c1b, new: tip+CL7400055

Benchmark results on darwin/amd64:
benchmark                                 old ns/op    new ns/op    delta
BenchmarkInterfaces-2                         34402        34231   -0.50%
BenchmarkInterfaceByIndex-2                   13192        12956   -1.79%
BenchmarkInterfaceByName-2                    34791        34388   -1.16%
BenchmarkInterfaceAddrs-2                     36565        63906  +74.77%
BenchmarkInterfacesAndAddrs-2                 17497        31068  +77.56%
BenchmarkInterfacesAndMulticastAddrs-2        25276        66711  +163.93%
old: 15797:9c3930413c1b, new: tip

Benchmark results on darwin/amd64:
benchmark                                 old ns/op    new ns/op    delta
BenchmarkInterfaces-2                         34402        31854   -7.41%
BenchmarkInterfaceByIndex-2                   13192        12950   -1.83%
BenchmarkInterfaceByName-2                    34791        31926   -8.23%
BenchmarkInterfaceAddrs-2                     36565        42144  +15.26%
BenchmarkInterfacesAndAddrs-2                 17497        17329   -0.96%
BenchmarkInterfacesAndMulticastAddrs-2        25276        24870   -1.61%
old: 15797:9c3930413c1b, new: tip+CL7400055

Update issue 4234.
Fixes issue 4839 (again).
Fixes issue 4866.

[mikio, 2013-02-27 05:29:29]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[albert.strasheim, 2013-02-27 07:10:22]

Some interesting stuff going on...

old: tip, new: tip+CL7400055
BenchmarkInterfaces-8                       4380009      4936148  +12.70%
BenchmarkInterfaceByIndex-8                 4529097      5456280  +20.47%
BenchmarkInterfaceByName-8                  4368222      4181615   -4.27%
BenchmarkInterfaceAddrs-8                 189645232      4781708  -97.48%
BenchmarkInterfacesAndAddrs-8             305493996        53603  -99.98%
BenchmarkInterfacesAndMulticastAddrs-8     16474957        62964  -99.62%

old: 15797:9c3930413c1b, new: tip+CL7400055

BenchmarkInterfaces-8                       4567956      4936148   +8.06%
BenchmarkInterfaceByIndex-8                 4245916      5456280  +28.51%
BenchmarkInterfaceByName-8                  4077000      4181615   +2.57%
BenchmarkInterfaceAddrs-8                     81632      4781708  +5757.64%
BenchmarkInterfacesAndAddrs-8                 51109        53603   +4.88%
BenchmarkInterfacesAndMulticastAddrs-8      3869755        62964  -98.37%

This is a pretty standard server, but it has 20 interfaces that show up in ip
link, probably due to some KVM virtual machine stuff.

I'll send you some more details in an email.

Maybe you need a binary search somewhere.

On 2013/02/27 05:29:29, mikio wrote:
> Hello mailto:golang-dev@googlegroups.com,
> 
> I'd like you to review this change to
> https://code.google.com/p/go

interfaceTableCache might not be the best name given the legacy of the previous
CL.

As far as I can see, you're just looking for a type to add byIndex to?

How about just calling it interfaceTables?

[mikio, 2013-02-27 07:38:20]

On Wed, Feb 27, 2013 at 4:10 PM,  <fullung@gmail.com> wrote:

> This is a pretty standard server, but it has 20 interfaces that show up
> in ip link, probably due to some KVM virtual machine stuff.
>
> I'll send you some more details in an email.

Thanks.

> Maybe you need a binary search somewhere.

I suppose that the users use network interface API for fetching
the kernel state at their applications start point. Then they starts
maintaining own tables using notifications via netlink or routing
sockets.

Perhaps we need a helper package to mitigate such boilerplate
kernel communication stuff but it would be go.net/route or similar
package.

Does this make sense to you?

> interfaceTableCache might not be the best name given the legacy of the
> previous CL.
>
> As far as I can see, you're just looking for a type to add byIndex to?
>
> How about just calling it interfaceTables?

Thanks, I'm not good at naming things.
At first it was just "type memo []Interface".

[albert.strasheim, 2013-02-27 08:44:31]

Hello

On 2013/02/27 07:38:20, mikio wrote:
> On Wed, Feb 27, 2013 at 4:10 PM,  <mailto:fullung@gmail.com> wrote:
> > This is a pretty standard server, but it has 20 interfaces that show up
> > in ip link, probably due to some KVM virtual machine stuff.
> > I'll send you some more details in an email.
> Thanks.
> > Maybe you need a binary search somewhere.
> I suppose that the users use network interface API for fetching
> the kernel state at their applications start point. Then they starts
> maintaining own tables using notifications via netlink or routing
> sockets.

Do I understand correctly that the old code in 9c3930413c1b somehow avoided ever
making the system calls more than once and that the new code makes the system
calls each time, but that this avoids all the caching uneasiness from the
previous CL?

As far as I can see, BenchmarkInterfaceAddrs is spending all its time making the
system calls, and its ns/op seems to correspond to the kind of timings you get
from stracing ip addr, so then although it's much slower than it was, it's as
fast as it can be.

Is this right?

> Perhaps we need a helper package to mitigate such boilerplate
> kernel communication stuff but it would be go.net/route or similar
> package.
> 
> Does this make sense to you?

Sounds potentially useful. I was just wondering why BenchmarkInterfaceAddrs got
so slow, but hopefully what I wrote above explains it.

Cheers

Albert

[mikio, 2013-02-27 08:45:56]

On Wed, Feb 27, 2013 at 4:38 PM, Mikio Hara <mikioh.mikioh@gmail.com> wrote:

>> Maybe you need a binary search somewhere.
(snip)
> Does this make sense to you?

What I mean... thanks, will investigate tonight.
Well, does this CL fix your IPoIB lookup problem?

[albert.strasheim, 2013-02-27 08:47:55]

Hello

On 2013/02/27 08:45:56, mikio wrote:
> On Wed, Feb 27, 2013 at 4:38 PM, Mikio Hara <mailto:mikioh.mikioh@gmail.com>
wrote:
> >> Maybe you need a binary search somewhere.
> (snip)
> > Does this make sense to you?
> What I mean... thanks, will investigate tonight.
> Well, does this CL fix your IPoIB lookup problem?

I think we've crossed messages.

I think it fixes it and this CL is as fast as it can be (if I understand the
problem correctly), so LGTM.

Cheers

Albert

[mikio, 2013-02-28 01:31:51]

On Wed, Feb 27, 2013 at 5:44 PM,  <fullung@gmail.com> wrote:

> Do I understand correctly that the old code in 9c3930413c1b somehow
> avoided ever making the system calls more than once and that the new
> code makes the system calls each time, but that this avoids all the
> caching uneasiness from the previous CL?

Yes.

> As far as I can see, BenchmarkInterfaceAddrs is spending all its time
> making the system calls, and its ns/op seems to correspond to the kind
> of timings you get from stracing ip addr, so then although it's much
> slower than it was, it's as fast as it can be.
>
> Is this right?

Right. Previous InterfaceAddrs doesn't need to lookup interfaces but at tip,
a) On linux, it's required to identify whether the address belongs to point-to-
point link, b) On all platforms supporting IPv6, to identify IPv6 scoped
addressing zone.

> Sounds potentially useful. I was just wondering why
> BenchmarkInterfaceAddrs got so slow, but hopefully what I wrote above
> explains it.

Thanks. Also I didn't get the same situation you had;

> old: tip, new: tip+CL7400055
> benchmark                                 old ns/op    new ns/op    delta
> BenchmarkInterfaces-8                       4380009      4936148 +12.70%
> BenchmarkInterfaceByIndex-8                 4529097      5456280 +20.47%

perhaps, it depends on numbers of CPU core, virtual machine runtime.

[mikio, 2013-02-28 01:33:40]

> I think it fixes it and this CL is as fast as it can be (if I understand the
> problem correctly), so LGTM.

Thanks, will submit later.

[mikio, 2013-02-28 05:58:50]

*** Submitted as https://code.google.com/p/go/source/detail?r=0b4084d8be42 ***

net: fix slow network interface manipulations

This CL reduces unnecessary network facility lookups introduced
by recent changes below.

changeset: 15798:53a4da6a4f4a
net: return correct point-to-point interface address on linux

changeset: 15799:a81ef8e0cc05
net: set up IPv6 scoped addressing zone for network facilities

Also adds a test case for issue	4839.

Benchmark results on linux/amd64, virtual machine:
benchmark                                 old ns/op    new ns/op    delta
BenchmarkInterfaces-2                         80487        80382   -0.13%
BenchmarkInterfaceByIndex-2                   72013        71391   -0.86%
BenchmarkInterfaceByName-2                    79865        80101   +0.30%
BenchmarkInterfaceAddrs-2                     42071       829677  +1872.09%
BenchmarkInterfacesAndAddrs-2                 35016       607622  +1635.27%
BenchmarkInterfacesAndMulticastAddrs-2       169849       169082   -0.45%
old: 15797:9c3930413c1b, new: tip

Benchmark results on linux/amd64, virtual machine:
benchmark                                 old ns/op    new ns/op    delta
BenchmarkInterfaces-2                         80487        81459   +1.21%
BenchmarkInterfaceByIndex-2                   72013        71512   -0.70%
BenchmarkInterfaceByName-2                    79865        80567   +0.88%
BenchmarkInterfaceAddrs-2                     42071       120108  +185.49%
BenchmarkInterfacesAndAddrs-2                 35016        33259   -5.02%
BenchmarkInterfacesAndMulticastAddrs-2       169849        82391  -51.49%
old: 15797:9c3930413c1b, new: tip+CL7400055

Benchmark results on darwin/amd64:
benchmark                                 old ns/op    new ns/op    delta
BenchmarkInterfaces-2                         34402        34231   -0.50%
BenchmarkInterfaceByIndex-2                   13192        12956   -1.79%
BenchmarkInterfaceByName-2                    34791        34388   -1.16%
BenchmarkInterfaceAddrs-2                     36565        63906  +74.77%
BenchmarkInterfacesAndAddrs-2                 17497        31068  +77.56%
BenchmarkInterfacesAndMulticastAddrs-2        25276        66711  +163.93%
old: 15797:9c3930413c1b, new: tip

Benchmark results on darwin/amd64:
benchmark                                 old ns/op    new ns/op    delta
BenchmarkInterfaces-2                         34402        31854   -7.41%
BenchmarkInterfaceByIndex-2                   13192        12950   -1.83%
BenchmarkInterfaceByName-2                    34791        31926   -8.23%
BenchmarkInterfaceAddrs-2                     36565        42144  +15.26%
BenchmarkInterfacesAndAddrs-2                 17497        17329   -0.96%
BenchmarkInterfacesAndMulticastAddrs-2        25276        24870   -1.61%
old: 15797:9c3930413c1b, new: tip+CL7400055

Update issue 4234.
Fixes issue 4839 (again).
Fixes issue 4866.

R=golang-dev, fullung
CC=golang-dev
https://codereview.appspot.com/7400055