FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

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


Description:
net: speed up network interface manipulations by caching

This CL introduces internal network interface lists to avoid
network interface manipulation slow down caused by recent
changes below.

changeset: 15798:53a4da6a4f4a
changeset: 15799:a81ef8e0cc05

Results on linux/amd64, virtual machine:
benchmark old ns/op new ns/op
delta
BenchmarkInterfaces 80652 81753
+1.37%
BenchmarkInterfaceByIndex 71534 245
-99.66%
BenchmarkInterfaceByName 80563 216
-99.73%
BenchmarkInterfaceAddrs 841526 45642
-94.58%
BenchmarkInterfacesAndAddrs 615326 37110
-93.97%
BenchmarkInterfacesAndMulticastAddrs 172234 91853
-46.67%

Results on darwin/amd64:
benchmark old ns/op new ns/op
delta
BenchmarkInterfaces 34709 37438
+7.86%
BenchmarkInterfaceByIndex 13168 242
-98.16%
BenchmarkInterfaceByName 37281 228
-99.39%
BenchmarkInterfaceAddrs 66431 37463
-43.61%
BenchmarkInterfacesAndAddrs 30903 17992
-41.78%
BenchmarkInterfacesAndMulticastAddrs 67520 26916
-60.14%

Fixes issue 4866.

Please review this at https://codereview.appspot.com/7384048/

Affected files:
M src/pkg/net/interface.go
M src/pkg/net/interface_linux.go
M src/pkg/net/ipsock.go


Index: src/pkg/net/interface.go
===================================================================
--- a/src/pkg/net/interface.go
+++ b/src/pkg/net/interface.go
@@ -6,7 +6,10 @@

package net

-import "errors"
+import (
+ "errors"
+ "sync"
+)

var (
errInvalidInterface = errors.New("net: invalid interface")
@@ -16,6 +19,32 @@
errNoSuchMulticastInterface = errors.New("net: no such multicast
interface")
)

+var (
+ interfaceMapLock sync.RWMutex
+ interfaceIndexMap = make(map[int]*Interface)
+ interfaceNameMap = make(map[string]*Interface)
+)
+
+func interfaceByIndex(index int) (*Interface, error) {
+ interfaceMapLock.RLock()
+ defer interfaceMapLock.RUnlock()
+ ifi, ok := interfaceIndexMap[index]
+ if !ok {
+ return nil, errNoSuchInterface
+ }
+ return ifi, nil
+}
+
+func interfaceByName(name string) (*Interface, error) {
+ interfaceMapLock.RLock()
+ defer interfaceMapLock.RUnlock()
+ ifi, ok := interfaceNameMap[name]
+ if !ok {
+ return nil, errNoSuchInterface
+ }
+ return ifi, nil
+}
+
// Interface represents a mapping between network interface name
// and index. It also represents network interface facility
// information.
@@ -78,9 +107,21 @@
return interfaceMulticastAddrTable(ifi.Index)
}

-// Interfaces returns a list of the system's network interfaces.
+// Interfaces returns a list of the system's network interfaces. It
+// also updates the internal list for reuse by other network interface
+// manipulations.
func Interfaces() ([]Interface, error) {
- return interfaceTable(0)
+ ift, err := interfaceTable(0)
+ if err != nil {
+ return nil, err
+ }
+ interfaceMapLock.Lock()
+ defer interfaceMapLock.Unlock()
+ for i := range ift {
+ interfaceIndexMap[ift[i].Index] = &ift[i]
+ interfaceNameMap[ift[i].Name] = &ift[i]
+ }
+ return ift, nil
}

// InterfaceAddrs returns a list of the system's network interface
@@ -94,14 +135,7 @@
if index <= 0 {
return nil, errInvalidInterfaceIndex
}
- ift, err := interfaceTable(index)
- if err != nil {
- return nil, err
- }
- for _, ifi := range ift {
- return &ifi, nil
- }
- return nil, errNoSuchInterface
+ return interfaceByIndex(index)
}

// InterfaceByName returns the interface specified by name.
@@ -109,14 +143,5 @@
if name == "" {
return nil, errInvalidInterfaceName
}
- ift, err := interfaceTable(0)
- if err != nil {
- return nil, err
- }
- for _, ifi := range ift {
- if name == ifi.Name {
- return &ifi, nil
- }
- }
- return nil, errNoSuchInterface
+ return interfaceByName(name)
}
Index: src/pkg/net/interface_linux.go
===================================================================
--- a/src/pkg/net/interface_linux.go
+++ b/src/pkg/net/interface_linux.go
@@ -116,15 +116,15 @@
break loop
case syscall.RTM_NEWADDR:
ifam := (*syscall.IfAddrmsg)(unsafe.Pointer(&m.Data[0]))
- ifi, err := InterfaceByIndex(int(ifam.Index))
- if err != nil {
- return nil, err
- }
if ifindex == 0 || ifindex == int(ifam.Index) {
attrs, err := syscall.ParseNetlinkRouteAttr(&m)
if err != nil {
return nil, os.NewSyscallError("netlink routeattr", err)
}
+ ifi, err := interfaceByIndex(int(ifam.Index))
+ if err != nil {
+ return nil, err
+ }
ifat = append(ifat, newAddr(attrs, ifi, ifam))
}
}
Index: src/pkg/net/ipsock.go
===================================================================
--- a/src/pkg/net/ipsock.go
+++ b/src/pkg/net/ipsock.go
@@ -13,6 +13,7 @@
func init() {
sysInit()
supportsIPv6, supportsIPv4map = probeIPv6Stack()
+ Interfaces()
}

func firstFavoriteAddr(filter func(IP) IP, addrs []string) (addr IP) {
@@ -211,7 +212,7 @@
if zone == 0 {
return ""
}
- if ifi, err := InterfaceByIndex(zone); err == nil {
+ if ifi, err := interfaceByIndex(zone); err == nil {
return ifi.Name
}
return itod(uint(zone))
@@ -221,7 +222,7 @@
if zone == "" {
return 0
}
- if ifi, err := InterfaceByName(zone); err == nil {
+ if ifi, err := interfaceByName(zone); err == nil {
return ifi.Index
}
n, _, _ := dtoi(zone, 0)


--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Mikioh Mikioh at Feb 21, 2013 at 5:12 pm
    Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

    Please take another look.


    https://codereview.appspot.com/7384048/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Brad Fitzpatrick at Feb 21, 2013 at 5:12 pm
    what type of code cares about this? I'd rather have accurate, uncached
    Interfaces() than super-fast cached calls.

    when is this cache invalidated?

    On Thu, Feb 21, 2013 at 8:57 AM, wrote:

    Reviewers: golang-dev_googlegroups.com,

    Message:
    Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

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


    Description:
    net: speed up network interface manipulations by caching

    This CL introduces internal network interface lists to avoid
    network interface manipulation slow down caused by recent
    changes below.

    changeset: 15798:53a4da6a4f4a
    changeset: 15799:a81ef8e0cc05

    Results on linux/amd64, virtual machine:
    benchmark old ns/op new ns/op
    delta
    BenchmarkInterfaces 80652 81753
    +1.37%
    BenchmarkInterfaceByIndex 71534 245
    -99.66%
    BenchmarkInterfaceByName 80563 216
    -99.73%
    BenchmarkInterfaceAddrs 841526 45642
    -94.58%
    BenchmarkInterfacesAndAddrs 615326 37110
    -93.97%
    BenchmarkInterfacesAndMulticas**tAddrs 172234 91853
    -46.67%

    Results on darwin/amd64:
    benchmark old ns/op new ns/op
    delta
    BenchmarkInterfaces 34709 37438
    +7.86%
    BenchmarkInterfaceByIndex 13168 242
    -98.16%
    BenchmarkInterfaceByName 37281 228
    -99.39%
    BenchmarkInterfaceAddrs 66431 37463
    -43.61%
    BenchmarkInterfacesAndAddrs 30903 17992
    -41.78%
    BenchmarkInterfacesAndMulticas**tAddrs 67520 26916
    -60.14%

    Fixes issue 4866.

    Please review this at https://codereview.appspot.**com/7384048/<https://codereview.appspot.com/7384048/>

    Affected files:
    M src/pkg/net/interface.go
    M src/pkg/net/interface_linux.go
    M src/pkg/net/ipsock.go


    Index: src/pkg/net/interface.go
    ==============================**==============================**=======
    --- a/src/pkg/net/interface.go
    +++ b/src/pkg/net/interface.go
    @@ -6,7 +6,10 @@

    package net

    -import "errors"
    +import (
    + "errors"
    + "sync"
    +)

    var (
    errInvalidInterface = errors.New("net: invalid interface")
    @@ -16,6 +19,32 @@
    errNoSuchMulticastInterface = errors.New("net: no such multicast
    interface")
    )

    +var (
    + interfaceMapLock sync.RWMutex
    + interfaceIndexMap = make(map[int]*Interface)
    + interfaceNameMap = make(map[string]*Interface)
    +)
    +
    +func interfaceByIndex(index int) (*Interface, error) {
    + interfaceMapLock.RLock()
    + defer interfaceMapLock.RUnlock()
    + ifi, ok := interfaceIndexMap[index]
    + if !ok {
    + return nil, errNoSuchInterface
    + }
    + return ifi, nil
    +}
    +
    +func interfaceByName(name string) (*Interface, error) {
    + interfaceMapLock.RLock()
    + defer interfaceMapLock.RUnlock()
    + ifi, ok := interfaceNameMap[name]
    + if !ok {
    + return nil, errNoSuchInterface
    + }
    + return ifi, nil
    +}
    +
    // Interface represents a mapping between network interface name
    // and index. It also represents network interface facility
    // information.
    @@ -78,9 +107,21 @@
    return interfaceMulticastAddrTable(**ifi.Index)
    }

    -// Interfaces returns a list of the system's network interfaces.
    +// Interfaces returns a list of the system's network interfaces. It
    +// also updates the internal list for reuse by other network interface
    +// manipulations.
    func Interfaces() ([]Interface, error) {
    - return interfaceTable(0)
    + ift, err := interfaceTable(0)
    + if err != nil {
    + return nil, err
    + }
    + interfaceMapLock.Lock()
    + defer interfaceMapLock.Unlock()
    + for i := range ift {
    + interfaceIndexMap[ift[i].**Index] = &ift[i]
    + interfaceNameMap[ift[i].Name] = &ift[i]
    + }
    + return ift, nil
    }

    // InterfaceAddrs returns a list of the system's network interface
    @@ -94,14 +135,7 @@
    if index <= 0 {
    return nil, errInvalidInterfaceIndex
    }
    - ift, err := interfaceTable(index)
    - if err != nil {
    - return nil, err
    - }
    - for _, ifi := range ift {
    - return &ifi, nil
    - }
    - return nil, errNoSuchInterface
    + return interfaceByIndex(index)
    }

    // InterfaceByName returns the interface specified by name.
    @@ -109,14 +143,5 @@
    if name == "" {
    return nil, errInvalidInterfaceName
    }
    - ift, err := interfaceTable(0)
    - if err != nil {
    - return nil, err
    - }
    - for _, ifi := range ift {
    - if name == ifi.Name {
    - return &ifi, nil
    - }
    - }
    - return nil, errNoSuchInterface
    + return interfaceByName(name)
    }
    Index: src/pkg/net/interface_linux.go
    ==============================**==============================**=======
    --- a/src/pkg/net/interface_linux.**go
    +++ b/src/pkg/net/interface_linux.**go
    @@ -116,15 +116,15 @@
    break loop
    case syscall.RTM_NEWADDR:
    ifam := (*syscall.IfAddrmsg)(unsafe.**
    Pointer(&m.Data[0]))
    - ifi, err := InterfaceByIndex(int(ifam.**Index))
    - if err != nil {
    - return nil, err
    - }
    if ifindex == 0 || ifindex == int(ifam.Index) {
    attrs, err :=
    syscall.ParseNetlinkRouteAttr(**&m)
    if err != nil {
    return nil,
    os.NewSyscallError("netlink routeattr", err)
    }
    + ifi, err := interfaceByIndex(int(ifam.**
    Index))
    + if err != nil {
    + return nil, err
    + }
    ifat = append(ifat, newAddr(attrs, ifi,
    ifam))
    }
    }
    Index: src/pkg/net/ipsock.go
    ==============================**==============================**=======
    --- a/src/pkg/net/ipsock.go
    +++ b/src/pkg/net/ipsock.go
    @@ -13,6 +13,7 @@
    func init() {
    sysInit()
    supportsIPv6, supportsIPv4map = probeIPv6Stack()
    + Interfaces()
    }

    func firstFavoriteAddr(filter func(IP) IP, addrs []string) (addr IP) {
    @@ -211,7 +212,7 @@
    if zone == 0 {
    return ""
    }
    - if ifi, err := InterfaceByIndex(zone); err == nil {
    + if ifi, err := interfaceByIndex(zone); err == nil {
    return ifi.Name
    }
    return itod(uint(zone))
    @@ -221,7 +222,7 @@
    if zone == "" {
    return 0
    }
    - if ifi, err := InterfaceByName(zone); err == nil {
    + if ifi, err := interfaceByName(zone); err == nil {
    return ifi.Index
    }
    n, _, _ := dtoi(zone, 0)


    --

    ---You received this message because you are subscribed to the Google
    Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an
    email to golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegroups.com>
    .
    For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/opt_out>
    .

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Bradfitz at Feb 21, 2013 at 5:16 pm
    https://codereview.appspot.com/7384048/diff/11001/src/pkg/net/interface.go
    File src/pkg/net/interface.go (right):

    https://codereview.appspot.com/7384048/diff/11001/src/pkg/net/interface.go#newcode110
    src/pkg/net/interface.go:110: // Interfaces returns a list of the
    system's network interfaces. It
    drop this doc addition. it's an internal detail. callers don't care or
    need to know.

    https://codereview.appspot.com/7384048/diff/11001/src/pkg/net/interface.go#newcode138
    src/pkg/net/interface.go:138: return interfaceByIndex(index)
    shouldn't this still work if the user never called Interfaces() first to
    populate the map?

    https://codereview.appspot.com/7384048/diff/11001/src/pkg/net/interface.go#newcode146
    src/pkg/net/interface.go:146: return interfaceByName(name)
    likewise.

    also, on miss, it should re-call Interfaces() to re-seed the map.

    what if I look at Interfaces(), create a bridge with
    os/exec.Command("brctl"), and then try to call InterfacesByName("br0")
    and it's not there? but I know it is, because I just made it. so you
    should assume that you failed on a miss.

    https://codereview.appspot.com/7384048/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Mikio Hara at Feb 21, 2013 at 5:44 pm

    On Fri, Feb 22, 2013 at 2:16 AM, wrote:

    also, on miss, it should re-call Interfaces() to re-seed the map.

    what if I look at Interfaces(), create a bridge with
    os/exec.Command("brctl"), and then try to call InterfacesByName("br0")
    and it's not there? but I know it is, because I just made it. so you
    should assume that you failed on a miss.
    yup, right. let me sleep on it; either receiving notifications from the
    kernel via netlink, routing sockets or taking accuracy as you suggest
    in previous comments.

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedFeb 21, '13 at 4:57p
activeFeb 21, '13 at 5:44p
posts5
users2
websitegolang.org

2 users in discussion

Mikio Hara: 3 posts Bradfitz: 2 posts

People

Translate

site design / logo © 2022 Grokbase