Skip to content

Commit

Permalink
endpoint: avoid excessive allocations during restore
Browse files Browse the repository at this point in the history
The way endpoint state currently is restored from header files could
potentially lead to a lot of memory allocations, partially due to the fact
that the respective base64 encoded state is converted to a string after
being read and then converted back to a byte slice in order to decode
it. Since every conversion of a byte slice to a string allocates (due to
the fact that strings are immutable in Go), this essentially doubles the
memory that is used which could lead to memory allocation spikes during
restore.

Avoid this by reading the base64 encoded endpoint state into a
byte slice directly and thus reducing the size and number of
allocations.

Before:

  EndpointSuite.BenchmarkReadEPsFromDirNames	    5000	    399980 ns/op	   83665 B/op	     743 allocs/op

After:

  EndpointSuite.BenchmarkReadEPsFromDirNames	    5000	    369643 ns/op	   73479 B/op	     731 allocs/op

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
  • Loading branch information
tklauser committed Jul 3, 2020
1 parent 7e3af79 commit 23e7952
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 19 deletions.
22 changes: 12 additions & 10 deletions pkg/endpoint/endpoint.go
Expand Up @@ -15,6 +15,7 @@
package endpoint

import (
"bytes"
"context"
"encoding/base64"
"encoding/json"
Expand Down Expand Up @@ -738,14 +739,15 @@ func (e *Endpoint) base64() (string, error) {
return base64.StdEncoding.EncodeToString(jsonBytes), nil
}

// parseBase64ToEndpoint parses the endpoint stored in the given base64 string.
func parseBase64ToEndpoint(str string, ep *Endpoint) error {
jsonBytes, err := base64.StdEncoding.DecodeString(str)
// parseBase64ToEndpoint parses the endpoint stored in the given base64 byte slice.
func parseBase64ToEndpoint(b []byte, ep *Endpoint) error {
jsonBytes := make([]byte, base64.StdEncoding.DecodedLen(len(b)))
n, err := base64.StdEncoding.Decode(jsonBytes, b)
if err != nil {
return err
}

if err := json.Unmarshal(jsonBytes, ep); err != nil {
if err := json.Unmarshal(jsonBytes[:n], ep); err != nil {
return fmt.Errorf("error unmarshaling serializableEndpoint from base64 representation: %s", err)
}

Expand All @@ -766,22 +768,22 @@ func FilterEPDir(dirFiles []os.FileInfo) []string {
return eptsID
}

// parseEndpoint parses the given strEp which is in the form of:
// parseEndpoint parses the given bEp which is in the form of:
// common.CiliumCHeaderPrefix + common.Version + ":" + endpointBase64
// Note that the parse'd endpoint's identity is only partially restored. The
// caller must call `SetIdentity()` to make the returned endpoint's identity useful.
func parseEndpoint(ctx context.Context, owner regeneration.Owner, strEp string) (*Endpoint, error) {
func parseEndpoint(ctx context.Context, owner regeneration.Owner, bEp []byte) (*Endpoint, error) {
// TODO: Provide a better mechanism to update from old version once we bump
// TODO: cilium version.
strEpSlice := strings.Split(strEp, ":")
if len(strEpSlice) != 2 {
return nil, fmt.Errorf("invalid format %q. Should contain a single ':'", strEp)
epSlice := bytes.Split(bEp, []byte{':'})
if len(epSlice) != 2 {
return nil, fmt.Errorf("invalid format %q. Should contain a single ':'", bEp)
}
ep := Endpoint{
owner: owner,
}

if err := parseBase64ToEndpoint(strEpSlice[1], &ep); err != nil {
if err := parseBase64ToEndpoint(epSlice[1], &ep); err != nil {
return nil, fmt.Errorf("failed to parse restored endpoint: %s", err)
}

Expand Down
19 changes: 10 additions & 9 deletions pkg/endpoint/restore.go
Expand Up @@ -16,6 +16,7 @@ package endpoint

import (
"bufio"
"bytes"
"context"
"encoding/json"
"fmt"
Expand Down Expand Up @@ -43,23 +44,23 @@ import (
)

// getCiliumVersionString returns the first line containing common.CiliumCHeaderPrefix.
func getCiliumVersionString(epCHeaderFilePath string) (string, error) {
func getCiliumVersionString(epCHeaderFilePath string) ([]byte, error) {
f, err := os.Open(epCHeaderFilePath)
if err != nil {
return "", err
return []byte{}, err
}
br := bufio.NewReader(f)
defer f.Close()
for {
s, err := br.ReadString('\n')
b, err := br.ReadBytes('\n')
if err == io.EOF {
return "", nil
return []byte{}, nil
}
if err != nil {
return "", err
return []byte{}, err
}
if strings.Contains(s, common.CiliumCHeaderPrefix) {
return s, nil
if bytes.Contains(b, []byte(common.CiliumCHeaderPrefix)) {
return b, nil
}
}
}
Expand Down Expand Up @@ -152,12 +153,12 @@ func ReadEPsFromDirNames(ctx context.Context, owner regeneration.Owner, basePath

scopedLog.Debug("Found endpoint C header file")

strEp, err := getCiliumVersionString(cHeaderFile)
bEp, err := getCiliumVersionString(cHeaderFile)
if err != nil {
scopedLog.WithError(err).Warn("Unable to read the C header file")
continue
}
ep, err := parseEndpoint(ctx, owner, strEp)
ep, err := parseEndpoint(ctx, owner, bEp)
if err != nil {
scopedLog.WithError(err).Warn("Unable to parse the C header file")
continue
Expand Down
40 changes: 40 additions & 0 deletions pkg/endpoint/restore_test.go
Expand Up @@ -229,6 +229,46 @@ func (ds *EndpointSuite) TestReadEPsFromDirNamesWithRestoreFailure(c *C) {
c.Assert(fileExists(fullDirName), checker.Equals, true)
}

func (ds *EndpointSuite) BenchmarkReadEPsFromDirNames(c *C) {
c.StopTimer()

// For this benchmark, the real linux datapath is necessary to properly
// serialize config files to disk and benchmark the restore.
oldDatapath := ds.datapath
defer func() {
ds.datapath = oldDatapath
}()
ds.datapath = linuxDatapath.NewDatapath(linuxDatapath.DatapathConfiguration{}, nil)

epsWanted, _ := ds.createEndpoints()
tmpDir, err := ioutil.TempDir("", "cilium-tests")
defer func() {
os.RemoveAll(tmpDir)
}()

os.Chdir(tmpDir)
c.Assert(err, IsNil)
epsNames := []string{}
for _, ep := range epsWanted {
c.Assert(ep, NotNil)

fullDirName := filepath.Join(tmpDir, ep.DirectoryPath())
err := os.MkdirAll(fullDirName, 0777)
c.Assert(err, IsNil)

err = ep.writeHeaderfile(fullDirName)
c.Assert(err, IsNil)

epsNames = append(epsNames, ep.DirectoryPath())
}
c.StartTimer()

for i := 0; i < c.N; i++ {
eps := ReadEPsFromDirNames(context.TODO(), ds, tmpDir, epsNames)
c.Assert(len(eps), Equals, len(epsWanted))
}
}

func (ds *EndpointSuite) TestPartitionEPDirNamesByRestoreStatus(c *C) {
eptsDirNames := []string{
"4", "12", "12_next", "3_next", "5_next_fail", "5",
Expand Down

0 comments on commit 23e7952

Please sign in to comment.