Merge pull request #65815 from wojtek-t/kube_proxy_less_allocations

pkg/proxy/iptables/proxier_test.go

@@ -56,38 +56,6 @@ func checkAllLines(t *testing.T, table utiliptables.Table, save []byte, expected
 	}
 }
 
-func TestReadLinesFromByteBuffer(t *testing.T) {
-	testFn := func(byteArray []byte, expected []string) {
-		index := 0
-		readIndex := 0
-		for ; readIndex < len(byteArray); index++ {
-			line, n := utiliptables.ReadLine(readIndex, byteArray)
-			readIndex = n
-			if expected[index] != line {
-				t.Errorf("expected:%q, actual:%q", expected[index], line)
-			}
-		} // for
-		if readIndex < len(byteArray) {
-			t.Errorf("Byte buffer was only partially read. Buffer length is:%d, readIndex is:%d", len(byteArray), readIndex)
-		}
-		if index < len(expected) {
-			t.Errorf("All expected strings were not compared. expected arr length:%d, matched count:%d", len(expected), index-1)
-		}
-	}
-
-	byteArray1 := []byte("\n  Line 1  \n\n\n L ine4  \nLine 5 \n \n")
-	expected1 := []string{"", "Line 1", "", "", "L ine4", "Line 5", ""}
-	testFn(byteArray1, expected1)
-
-	byteArray1 = []byte("")
-	expected1 = []string{}
-	testFn(byteArray1, expected1)
-
-	byteArray1 = []byte("\n\n")
-	expected1 = []string{"", ""}
-	testFn(byteArray1, expected1)
-}
-
 func TestGetChainLines(t *testing.T) {
 	iptablesSave := `# Generated by iptables-save v1.4.7 on Wed Oct 29 14:56:01 2014
 	*nat

pkg/util/iptables/BUILD

@@ -36,7 +36,10 @@ go_library(
 
 go_test(
     name = "go_default_test",
-    srcs = ["iptables_test.go"],
+    srcs = [
+        "iptables_test.go",
+        "save_restore_test.go",
+    ],
     embed = [":go_default_library"],
     deps = select({
         "@io_bazel_rules_go//go/platform:linux": [

pkg/util/iptables/save_restore.go

@@ -17,10 +17,15 @@ limitations under the License.
 package iptables
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 )
 
+var (
+	commitBytes = []byte("COMMIT")
+)
+
 // MakeChainLine return an iptables-save/restore formatted chain line given a Chain
 func MakeChainLine(chain Chain) string {
 	return fmt.Sprintf(":%s - [0:0]", chain)
@@ -30,38 +35,41 @@ func MakeChainLine(chain Chain) string {
 // It returns a map of iptables.Chain to string where the string is the chain line from the save (with counters etc).
 func GetChainLines(table Table, save []byte) map[Chain]string {
 	chainsMap := make(map[Chain]string)
-	tablePrefix := "*" + string(table)
+	tablePrefix := []byte("*" + string(table))
 	readIndex := 0
 	// find beginning of table
 	for readIndex < len(save) {
-		line, n := ReadLine(readIndex, save)
+		line, n := readLine(readIndex, save)
 		readIndex = n
-		if strings.HasPrefix(line, tablePrefix) {
+		if bytes.HasPrefix(line, tablePrefix) {
 			break
 		}
 	}
 	// parse table lines
 	for readIndex < len(save) {
-		line, n := ReadLine(readIndex, save)
+		line, n := readLine(readIndex, save)
 		readIndex = n
 		if len(line) == 0 {
 			continue
 		}
-		if strings.HasPrefix(line, "COMMIT") || strings.HasPrefix(line, "*") {
+		if bytes.HasPrefix(line, commitBytes) || line[0] == '*' {
 			break
-		} else if strings.HasPrefix(line, "#") {
+		} else if line[0] == '#' {
 			continue
-		} else if strings.HasPrefix(line, ":") && len(line) > 1 {
+		} else if line[0] == ':' && len(line) > 1 {
 			// We assume that the <line> contains space - chain lines have 3 fields,
 			// space delimited. If there is no space, this line will panic.
-			chain := Chain(line[1:strings.Index(line, " ")])
-			chainsMap[chain] = line
+			//
+			// TODO: Try to avoid these allocations by reusing memory with the input.
+			lineString := string(line)
+			chain := Chain(line[1:strings.Index(lineString, " ")])
+			chainsMap[chain] = lineString
 		}
 	}
 	return chainsMap
 }
 
-func ReadLine(readIndex int, byteArray []byte) (string, int) {
+func readLine(readIndex int, byteArray []byte) ([]byte, int) {
 	currentReadIndex := readIndex
 
 	// consume left spaces
@@ -89,7 +97,7 @@ func ReadLine(readIndex int, byteArray []byte) (string, int) {
 		} else if (byteArray[currentReadIndex] == '\n') || (currentReadIndex == (len(byteArray) - 1)) {
 			// end of line or byte buffer is reached
 			if currentReadIndex <= leftTrimIndex {
-				return "", currentReadIndex + 1
+				return nil, currentReadIndex + 1
 			}
 			// set the rightTrimIndex
 			if rightTrimIndex == -1 {
@@ -100,11 +108,12 @@ func ReadLine(readIndex int, byteArray []byte) (string, int) {
 					rightTrimIndex = currentReadIndex + 1
 				}
 			}
-			return string(byteArray[leftTrimIndex:rightTrimIndex]), currentReadIndex + 1
+			// Avoid unnecessary allocation.
+			return byteArray[leftTrimIndex:rightTrimIndex], currentReadIndex + 1
 		} else {
 			// unset rightTrimIndex
 			rightTrimIndex = -1
 		}
 	}
-	return "", currentReadIndex
+	return nil, currentReadIndex
 }

pkg/util/iptables/save_restore_test.go

+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package iptables
+
+import (
+	"testing"
+)
+
+func TestReadLinesFromByteBuffer(t *testing.T) {
+	testFn := func(byteArray []byte, expected []string) {
+		index := 0
+		readIndex := 0
+		for ; readIndex < len(byteArray); index++ {
+			line, n := readLine(readIndex, byteArray)
+			readIndex = n
+			if expected[index] != string(line) {
+				t.Errorf("expected:%q, actual:%q", expected[index], line)
+			}
+		} // for
+		if readIndex < len(byteArray) {
+			t.Errorf("Byte buffer was only partially read. Buffer length is:%d, readIndex is:%d", len(byteArray), readIndex)
+		}
+		if index < len(expected) {
+			t.Errorf("All expected strings were not compared. expected arr length:%d, matched count:%d", len(expected), index-1)
+		}
+	}
+
+	byteArray1 := []byte("\n  Line 1  \n\n\n L ine4  \nLine 5 \n \n")
+	expected1 := []string{"", "Line 1", "", "", "L ine4", "Line 5", ""}
+	testFn(byteArray1, expected1)
+
+	byteArray1 = []byte("")
+	expected1 = []string{}
+	testFn(byteArray1, expected1)
+
+	byteArray1 = []byte("\n\n")
+	expected1 = []string{"", ""}
+	testFn(byteArray1, expected1)
+}