Skip to content

Commit

Permalink
pkg/policy/policy: Optimize SearchContext String()
Browse files Browse the repository at this point in the history
Use strings.Builder instead of fmt.Sprintf() and preallocate the size of
the string so that Go doesn't need to over-allocate if the string ends
up longer than what the buffer growth algorithm predicts.

issue reference : #19571

Result:

```
$ go test -v -run '^$' -bench 'BenchmarkSearchContextString' -benchmem ./pkg/policy > BenchmarkSearchContextString_old.txt
$ go test -v -run '^$' -bench 'BenchmarkSearchContextString' -benchmem ./pkg/policy > BenchmarkSearchContextString_new.txt
$ benchcmp BenchmarkSearchContextString_old.txt BenchmarkSearchContextString_new.txt
benchmark                          old ns/op     new ns/op     delta
BenchmarkSearchContextString-2     9336          5106          -45.31%

benchmark                          old allocs     new allocs     delta
BenchmarkSearchContextString-2     77             47             -38.96%

benchmark                          old bytes     new bytes     delta
BenchmarkSearchContextString-2     2736          1848          -32.46%
```

Signed-off-by: MikeLing <sabergeass@gmail.com>
  • Loading branch information
MikeLing authored and nebril committed Jun 10, 2022
1 parent 4fbf63e commit 5feb34e
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 12 deletions.
28 changes: 16 additions & 12 deletions pkg/policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package policy

import (
"fmt"
"io"
stdlog "log"
"strconv"
Expand Down Expand Up @@ -69,28 +68,33 @@ type SearchContext struct {
}

func (s *SearchContext) String() string {
from := []string{}
to := []string{}
dports := []string{}
from := make([]string, 0, len(s.From))
to := make([]string, 0, len(s.To))
dports := make([]string, 0, len(s.DPorts))
for _, fromLabel := range s.From {
from = append(from, fromLabel.String())
}
for _, toLabel := range s.To {
to = append(to, toLabel.String())
}
// We should avoid to use `fmt.Sprintf()` since
// it is well-known for not being opimal in terms of
// CPU and memory allocations.
// See https://github.com/cilium/cilium/issues/19571
for _, dport := range s.DPorts {
if dport.Name != "" {
dports = append(dports, fmt.Sprintf("%s/%s", dport.Name, dport.Protocol))
} else {
dports = append(dports, fmt.Sprintf("%d/%s", dport.Port, dport.Protocol))
dportStr := dport.Name
if dportStr == "" {
dportStr = strconv.FormatUint(uint64(dport.Port), 10)
}
dports = append(dports, dportStr+"/"+dport.Protocol)
}
ret := fmt.Sprintf("From: [%s]", strings.Join(from, ", "))
ret += fmt.Sprintf(" => To: [%s]", strings.Join(to, ", "))
fromStr := strings.Join(from, ", ")
toStr := strings.Join(to, ", ")
if len(dports) != 0 {
ret += fmt.Sprintf(" Ports: [%s]", strings.Join(dports, ", "))
dportStr := strings.Join(dports, ", ")
return "From: [" + fromStr + "] => To: [" + toStr + "] Ports: [" + dportStr + "]"
}
return ret
return "From: [" + fromStr + "] => To: [" + toStr + "]"
}

func (s *SearchContext) CallDepth() string {
Expand Down
101 changes: 101 additions & 0 deletions pkg/policy/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import (
"testing"

. "gopkg.in/check.v1"

"github.com/cilium/cilium/api/v1/models"
"github.com/cilium/cilium/pkg/labels"
)

// Hook up gocheck into the "go test" runner.
Expand All @@ -19,3 +22,101 @@ func Test(t *testing.T) {
type PolicyTestSuite struct{}

var _ = Suite(&PolicyTestSuite{})

func (ds *PolicyTestSuite) TestSearchContextString(c *C) {
for expected, sc := range map[string]SearchContext{
"From: [unspec:a, unspec:b, unspec:c] => To: [unspec:d, unspec:e, unspec:f] Ports: [HTTP/TCP, HTTPs/TCP]": {
Trace: 1,
Depth: 0,
From: labels.ParseLabelArray("a", "c", "b"),
To: labels.ParseLabelArray("d", "e", "f"),
DPorts: []*models.Port{
{
Name: "HTTP",
Port: 80,
Protocol: "TCP",
},
{
Name: "HTTPs",
Port: 442,
Protocol: "TCP",
},
},
rulesSelect: false,
},
"From: [unspec:a, unspec:b, unspec:c] => To: [unspec:d, unspec:e, unspec:f] Ports: [80/TCP, 442/TCP]": {
Trace: 1,
Depth: 0,
From: labels.ParseLabelArray("a", "c", "b"),
To: labels.ParseLabelArray("d", "e", "f"),
DPorts: []*models.Port{
{
Port: 80,
Protocol: "TCP",
},
{
Port: 442,
Protocol: "TCP",
},
},
rulesSelect: false,
},
"From: [k8s:a, local:b, unspec:c] => To: [unspec:d, unspec:e, unspec:f]": {
Trace: 1,
Depth: 0,
From: labels.ParseLabelArray("k8s:a", "unspec:c", "local:b"),
To: labels.ParseLabelArray("d", "e", "f"),
rulesSelect: false,
},
} {
str := sc.String()
c.Assert(str, Equals, expected)
}
}

func BenchmarkSearchContextString(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
for _, sc := range []SearchContext{
{
Trace: 1,
Depth: 0,
From: labels.ParseLabelArray("a", "c", "b"),
To: labels.ParseLabelArray("d", "e", "f"),
DPorts: []*models.Port{
{
Name: "HTTP",
Port: 80,
Protocol: "TCP",
},
{
Name: "HTTPs",
Port: 442,
Protocol: "TCP",
},
},
rulesSelect: false,
},
{
Trace: 1,
Depth: 0,
From: labels.ParseLabelArray("a", "c", "b"),
To: labels.ParseLabelArray("d", "e", "f"),
DPorts: []*models.Port{
{
Port: 80,
Protocol: "TCP",
},
{
Port: 442,
Protocol: "TCP",
},
},
rulesSelect: false,
},
} {
_ = sc.String()
}
}
}

0 comments on commit 5feb34e

Please sign in to comment.