]> Cypherpunks repositories - gostls13.git/commitdiff
net: halve the allocs in ParseCIDR by sharing slice backing
authorChris Stockton <chrisstocktonaz@gmail.com>
Sun, 27 Oct 2019 15:55:53 +0000 (15:55 +0000)
committerBrad Fitzpatrick <bradfitz@golang.org>
Fri, 8 Nov 2019 18:25:40 +0000 (18:25 +0000)
Share a slice backing between the host address, network ip and mask.
Add tests to verify that each slice header has len==cap to prevent
introducing new behavior into Go programs. This has a small tradeoff
of allocating a larger slice backing when the address is invalid.
Earlier error detection of invalid prefix length helps balance this
cost and a new benchmark for ParseCIDR helps measure it.

This yields a ~22% speedup for all nil err cidr tests:

  name               old time/op    new time/op    delta
  ParseCIDR/IPv4-24    9.17µs ± 6%    7.20µs ± 7%  -21.47%  (p=0.000 n=20+20)
  ParseCIDR/IPv6-24    9.02µs ± 6%    6.95µs ± 9%  -23.02%  (p=0.000 n=20+20)
  ParseCIDR/IPv4-24    1.51kB ± 0%    1.55kB ± 0%   +2.65%  (p=0.000 n=20+20)
  ParseCIDR/IPv6-24    1.51kB ± 0%    1.55kB ± 0%   +2.65%  (p=0.000 n=20+20)
  ParseCIDR/IPv4-24      68.0 ± 0%      34.0 ± 0%  -50.00%  (p=0.000 n=20+20)
  ParseCIDR/IPv6-24      68.0 ± 0%      34.0 ± 0%  -50.00%  (p=0.000 n=20+20)

Including non-nil err cidr tests gains around 25%~:

  name               old time/op    new time/op    delta
  ParseCIDR/IPv4-24    11.8µs ±11%     8.9µs ± 8%  -24.88%  (p=0.000 n=20+20)
  ParseCIDR/IPv6-24    11.7µs ± 7%     8.7µs ± 5%  -25.93%  (p=0.000 n=20+20)
  ParseCIDR/IPv4-24    1.98kB ± 0%    2.00kB ± 0%   +1.21%  (p=0.000 n=20+20)
  ParseCIDR/IPv6-24    1.98kB ± 0%    2.00kB ± 0%   +1.21%  (p=0.000 n=20+20)
  ParseCIDR/IPv4-24      87.0 ± 0%      48.0 ± 0%  -44.83%  (p=0.000 n=20+20)
  ParseCIDR/IPv6-24      87.0 ± 0%      48.0 ± 0%  -44.83%  (p=0.000 n=20+20)

Change-Id: I17f33c9049f7875b6ebdfde1f80b386a7aef9b94
GitHub-Last-Rev: 0a031f44b458e2c6465d0e59fb4653e08c44a854
GitHub-Pull-Request: golang/go#26948
Reviewed-on: https://go-review.googlesource.com/c/go/+/129118
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
src/net/ip.go
src/net/ip_test.go

index 7a554869336a55c5dbfc3c153f8a0c8e8a876cba..a80851803d5c9c4fc02a19f135849aa3f126bc51 100644 (file)
@@ -75,10 +75,20 @@ func CIDRMask(ones, bits int) IPMask {
        if ones < 0 || ones > bits {
                return nil
        }
-       l := bits / 8
-       m := make(IPMask, l)
+       return putCIDRMask(nil, ones, bits/8)
+}
+
+// putCIDRMask will put an IPMask into `m` consisting of `ones` 1 bits
+// followed by 0s up to a total length of `bits' bits if the length
+// of m is < `l` bytes and returns the same slice m. If m did not have
+// sufficient length for the mask l a new m is returned instead.
+func putCIDRMask(m IPMask, ones, bits int) IPMask {
+       if len(m) < bits {
+               m = make(IPMask, bits)
+       }
+
        n := uint(ones)
-       for i := 0; i < l; i++ {
+       for i := 0; i < bits; i++ {
                if n >= 8 {
                        m[i] = 0xff
                        n -= 8
@@ -243,6 +253,13 @@ func allFF(b []byte) bool {
 
 // Mask returns the result of masking the IP address ip with mask.
 func (ip IP) Mask(mask IPMask) IP {
+       return ipMaskWithBuf(nil, ip, mask)
+}
+
+// ipMaskWithBuf implements the IP.Mask method, but containing an
+// opitional optional bufer to use for the return value. If the buf is
+// too small, a new one is allocated.
+func ipMaskWithBuf(buf, ip IP, mask IPMask) []byte {
        if len(mask) == IPv6len && len(ip) == IPv4len && allFF(mask[:12]) {
                mask = mask[12:]
        }
@@ -253,11 +270,13 @@ func (ip IP) Mask(mask IPMask) IP {
        if n != len(mask) {
                return nil
        }
-       out := make(IP, n)
+       if len(buf) < n {
+               buf = make(IP, n)
+       }
        for i := 0; i < n; i++ {
-               out[i] = ip[i] & mask[i]
+               buf[i] = ip[i] & mask[i]
        }
-       return out
+       return buf
 }
 
 // ubtoa encodes the string form of the integer v to dst[start:] and
@@ -534,28 +553,35 @@ func (n *IPNet) String() string {
 // Parse IPv4 address (d.d.d.d).
 func parseIPv4(s string) IP {
        var p [IPv4len]byte
+       if !parseIntoIPv4(p[:], s) {
+               return nil
+       }
+       return IPv4(p[0], p[1], p[2], p[3])
+}
+
+// parseIntoIPv4 parses s as an IPv4 address and parses it into
+// p, which must be at least IPv4len bytes long. It reports
+// whether the parse was successful.
+func parseIntoIPv4(p []byte, s string) bool {
        for i := 0; i < IPv4len; i++ {
                if len(s) == 0 {
                        // Missing octets.
-                       return nil
+                       return false
                }
                if i > 0 {
                        if s[0] != '.' {
-                               return nil
+                               return false
                        }
                        s = s[1:]
                }
                n, c, ok := dtoi(s)
                if !ok || n > 0xFF {
-                       return nil
+                       return false
                }
                s = s[c:]
                p[i] = byte(n)
        }
-       if len(s) != 0 {
-               return nil
-       }
-       return IPv4(p[0], p[1], p[2], p[3])
+       return len(s) == 0
 }
 
 // parseIPv6Zone parses s as a literal IPv6 address and its associated zone
@@ -568,7 +594,16 @@ func parseIPv6Zone(s string) (IP, string) {
 // parseIPv6 parses s as a literal IPv6 address described in RFC 4291
 // and RFC 5952.
 func parseIPv6(s string) (ip IP) {
-       ip = make(IP, IPv6len)
+       if ip = make(IP, IPv6len); !parseIntoIPv6(ip, s) {
+               ip = nil
+       }
+       return
+}
+
+// parseIntoIPv6 parses s as a literal IPv6 address described in RFC
+// 4291 and RFC 5952 and populates ip. It reports whether the parse
+// was successful.
+func parseIntoIPv6(ip IP, s string) bool {
        ellipsis := -1 // position of ellipsis in ip
 
        // Might have leading ellipsis
@@ -577,7 +612,7 @@ func parseIPv6(s string) (ip IP) {
                s = s[2:]
                // Might be only ellipsis
                if len(s) == 0 {
-                       return ip
+                       return true
                }
        }
 
@@ -587,22 +622,22 @@ func parseIPv6(s string) (ip IP) {
                // Hex number.
                n, c, ok := xtoi(s)
                if !ok || n > 0xFFFF {
-                       return nil
+                       return false
                }
 
                // If followed by dot, might be in trailing IPv4.
                if c < len(s) && s[c] == '.' {
                        if ellipsis < 0 && i != IPv6len-IPv4len {
                                // Not the right place.
-                               return nil
+                               return false
                        }
                        if i+IPv4len > IPv6len {
                                // Not enough room.
-                               return nil
+                               return false
                        }
                        ip4 := parseIPv4(s)
                        if ip4 == nil {
-                               return nil
+                               return false
                        }
                        ip[i] = ip4[12]
                        ip[i+1] = ip4[13]
@@ -626,14 +661,14 @@ func parseIPv6(s string) (ip IP) {
 
                // Otherwise must be followed by colon and more.
                if s[0] != ':' || len(s) == 1 {
-                       return nil
+                       return false
                }
                s = s[1:]
 
                // Look for ellipsis.
                if s[0] == ':' {
                        if ellipsis >= 0 { // already have one
-                               return nil
+                               return false
                        }
                        ellipsis = i
                        s = s[1:]
@@ -645,13 +680,13 @@ func parseIPv6(s string) (ip IP) {
 
        // Must have used entire string.
        if len(s) != 0 {
-               return nil
+               return false
        }
 
        // If didn't parse enough, expand ellipsis.
        if i < IPv6len {
                if ellipsis < 0 {
-                       return nil
+                       return false
                }
                n := IPv6len - i
                for j := i - 1; j >= ellipsis; j-- {
@@ -662,9 +697,9 @@ func parseIPv6(s string) (ip IP) {
                }
        } else if ellipsis >= 0 {
                // Ellipsis must represent at least one 0 group.
-               return nil
+               return false
        }
-       return ip
+       return true
 }
 
 // ParseIP parses s as an IP address, returning the result.
@@ -673,15 +708,29 @@ func parseIPv6(s string) (ip IP) {
 // If s is not a valid textual representation of an IP address,
 // ParseIP returns nil.
 func ParseIP(s string) IP {
+       switch n := strToIPvLen(s); n {
+       case IPv4len:
+               return parseIPv4(s)
+       case IPv6len:
+               return parseIPv6(s)
+       default:
+               return nil
+       }
+}
+
+// strToIPvLen reports the expected length of the parsed IP adress
+// given its ASCII representation in s. It returns IPv4len, IPv6len,
+// or -1.
+func strToIPvLen(s string) int {
        for i := 0; i < len(s); i++ {
                switch s[i] {
                case '.':
-                       return parseIPv4(s)
+                       return IPv4len
                case ':':
-                       return parseIPv6(s)
+                       return IPv6len
                }
        }
-       return nil
+       return -1
 }
 
 // parseIPZone parses s as an IP address, return it and its associated zone
@@ -711,17 +760,53 @@ func ParseCIDR(s string) (IP, *IPNet, error) {
        if i < 0 {
                return nil, nil, &ParseError{Type: "CIDR address", Text: s}
        }
-       addr, mask := s[:i], s[i+1:]
-       iplen := IPv4len
-       ip := parseIPv4(addr)
-       if ip == nil {
-               iplen = IPv6len
-               ip = parseIPv6(addr)
+
+       iplen := strToIPvLen(s[:i])
+       if iplen < 0 {
+               return nil, nil, &ParseError{Type: "CIDR address", Text: s}
        }
-       n, i, ok := dtoi(mask)
-       if ip == nil || !ok || i != len(mask) || n < 0 || n > 8*iplen {
+
+       nwlen, y, ok := dtoi(s[i+1:])
+       if !ok || y != len(s[i+1:]) || nwlen < 0 || nwlen > 8*iplen {
                return nil, nil, &ParseError{Type: "CIDR address", Text: s}
        }
-       m := CIDRMask(n, 8*iplen)
-       return ip, &IPNet{IP: ip.Mask(m), Mask: m}, nil
+
+       var (
+               buf []byte
+               ipn *IPNet
+       )
+       switch iplen {
+       case IPv4len:
+               var block struct {
+                       ipn IPNet
+                       arr [IPv6len + (IPv4len * 2)]byte
+               }
+               ipn = &block.ipn
+               buf = block.arr[:]
+               copy(buf, v4InV6Prefix)
+               if !parseIntoIPv4(buf[len(v4InV6Prefix):], s[:i]) {
+                       return nil, nil, &ParseError{Type: "CIDR address", Text: s}
+               }
+       case IPv6len:
+               var block struct {
+                       ipn IPNet
+                       arr [IPv6len * 3]byte
+               }
+               ipn = &block.ipn
+               buf = block.arr[:]
+               if !parseIntoIPv6(buf, s[:i]) {
+                       return nil, nil, &ParseError{Type: "CIDR address", Text: s}
+               }
+       default:
+               return nil, nil, &ParseError{Type: "CIDR address", Text: s}
+       }
+
+       ip := buf[:IPv6len:IPv6len]
+       buf = buf[IPv6len:]
+
+       ipn.Mask = putCIDRMask(buf[:iplen:iplen], nwlen, iplen)
+       buf = buf[iplen:]
+
+       ipn.IP = ipMaskWithBuf(buf[:iplen:iplen], ip, ipn.Mask)
+       return ip, ipn, nil
 }
index a5fc5e644a2b2717ab16b1005c586be8bb65d2d6..4e7f0cf01f8d37506add7413143e1033f54dcefb 100644 (file)
@@ -46,14 +46,18 @@ var parseIPTests = []struct {
 
 func TestParseIP(t *testing.T) {
        for _, tt := range parseIPTests {
-               if out := ParseIP(tt.in); !reflect.DeepEqual(out, tt.out) {
+               out := ParseIP(tt.in)
+               if !reflect.DeepEqual(out, tt.out) {
                        t.Errorf("ParseIP(%q) = %v, want %v", tt.in, out, tt.out)
                }
+               if exp, got := cap(out), len(out); exp != got {
+                       t.Fatalf("ParseIP(%q) cap %v; want %v", tt.in, exp, got)
+               }
                if tt.in == "" {
                        // Tested in TestMarshalEmptyIP below.
                        continue
                }
-               var out IP
+               out = nil
                if err := out.UnmarshalText([]byte(tt.in)); !reflect.DeepEqual(out, tt.out) || (tt.out == nil) != (err != nil) {
                        t.Errorf("IP.UnmarshalText(%q) = %v, %v, want %v", tt.in, out, err, tt.out)
                }
@@ -107,6 +111,7 @@ func BenchmarkParseIP(b *testing.B) {
        testHookUninstaller.Do(uninstallTestHooks)
 
        for i := 0; i < b.N; i++ {
+               b.ReportAllocs()
                for _, tt := range parseIPTests {
                        ParseIP(tt.in)
                }
@@ -366,12 +371,41 @@ func TestParseCIDR(t *testing.T) {
                if !reflect.DeepEqual(err, tt.err) {
                        t.Errorf("ParseCIDR(%q) = %v, %v; want %v, %v", tt.in, ip, net, tt.ip, tt.net)
                }
-               if err == nil && (!tt.ip.Equal(ip) || !tt.net.IP.Equal(net.IP) || !reflect.DeepEqual(net.Mask, tt.net.Mask)) {
+               if err != nil {
+                       continue
+               }
+               if !tt.ip.Equal(ip) || !tt.net.IP.Equal(net.IP) || !reflect.DeepEqual(net.Mask, tt.net.Mask) {
                        t.Errorf("ParseCIDR(%q) = %v, {%v, %v}; want %v, {%v, %v}", tt.in, ip, net.IP, net.Mask, tt.ip, tt.net.IP, tt.net.Mask)
                }
+               if exp, got := cap(ip), len(ip); exp != got {
+                       t.Fatalf("ParseCIDR(%q) ip cap %v; want %v", tt.in, exp, got)
+               }
+               if exp, got := cap(net.IP), len(net.IP); exp != got {
+                       t.Fatalf("ParseCIDR(%q) net.IP cap %v; want %v", tt.in, exp, got)
+               }
+               if exp, got := cap(net.Mask), len(net.Mask); exp != got {
+                       t.Fatalf("ParseCIDR(%q) net.Mask cap %v; want %v", tt.in, exp, got)
+               }
        }
 }
 
+func BenchmarkParseCIDR(b *testing.B) {
+       testHookUninstaller.Do(uninstallTestHooks)
+
+       b.Run("IPv4", func(b *testing.B) {
+               b.ReportAllocs()
+               for i := 0; i < b.N; i++ {
+                       ParseCIDR("135.104.0.1/24")
+               }
+       })
+       b.Run("IPv6", func(b *testing.B) {
+               b.ReportAllocs()
+               for i := 0; i < b.N; i++ {
+                       ParseCIDR("2001:DB8::1/48")
+               }
+       })
+}
+
 var ipNetContainsTests = []struct {
        ip  IP
        net *IPNet