Skip to content

Commit

Permalink
labels: don't alloc a buf per label for SortedList
Browse files Browse the repository at this point in the history
[ upstream commit c1e21d8 ]

SortedList appears prominently in both CPU and Heap pprofs when in a
scenario with an FQDN policy matching S3. This is because DNS requests
to S3 return short-lived DNS responses from a pool of many IPs, each of
which will receive a CIDR identitiy in cilium.

Since we furthermore call SortedList repeatedly for these CIDR
identities (represented as a set of CIDR labels containing all
super-CIDRs), avoiding ~32 buffer allocations per call is worth it.

Before:
Labels_SortedList-10    	1000000	    3079 ns/op	   504 B/op	    13 allocs/op
Labels_SortedListCIDRIDs-10    	  52702	    21417 ns/op	   3680 B/op	     41 allocs/op

After:
Labels_SortedList-10    	1000000	    2164 ns/op	   360 B/op	     3 allocs/op
Labels_SortedListCIDRIDs-10    	  72180	    15444 ns/op	   1624 B/op	      3 allocs/op

Benchstat:
                     │     old     │                 opt                 │
                     │   sec/op    │   sec/op     vs base                │
Labels_SortedList-10   3.279µ ± 6%   2.209µ ± 6%  -32.65% (p=0.000 n=10)

                     │    B/op    │    B/op     vs base                │
Labels_SortedList-10   504.0 ± 0%   360.0 ± 0%  -28.57% (p=0.000 n=10)
                     │  allocs/op  │ allocs/op   vs base                │
Labels_SortedList-10   13.000 ± 0%   3.000 ± 0%  -76.92% (p=0.000 n=10)

pkg: github.com/cilium/cilium/pkg/labels/cidr
                            │     old     │                 opt                 │
                            │   sec/op    │   sec/op     vs base                │
Labels_SortedListCIDRIDs-10   21.23µ ± 5%   14.89µ ± 9%  -29.87% (p=0.000 n=10)
                            │     B/op     │     B/op      vs base                │
Labels_SortedListCIDRIDs-10   3.594Ki ± 0%   1.586Ki ± 0%  -55.87% (p=0.000 n=10)
                            │  allocs/op  │ allocs/op   vs base                │
Labels_SortedListCIDRIDs-10   41.000 ± 0%   3.000 ± 0%  -92.68% (p=0.000 n=10)

Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
  • Loading branch information
bimmlerd authored and tklauser committed Feb 20, 2024
1 parent c0b715a commit f41d675
Showing 1 changed file with 25 additions and 3 deletions.
28 changes: 25 additions & 3 deletions pkg/labels/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,13 +480,24 @@ func (l Label) FormatForKVStore() []byte {
// kvstore.prefixMatchesKey())
b := make([]byte, 0, len(l.Source)+len(l.Key)+len(l.Value)+3)
buf := bytes.NewBuffer(b)
l.formatForKVStoreInto(buf)
return buf.Bytes()
}

// formatForKVStoreInto writes the label as a formatted string, ending in
// a semicolon into buf.
//
// DO NOT BREAK THE FORMAT OF THIS. THE RETURNED STRING IS USED AS
// PART OF THE KEY IN THE KEY-VALUE STORE.
//
// Non-pointer receiver allows this to be called on a value in a map.
func (l Label) formatForKVStoreInto(buf *bytes.Buffer) {
buf.WriteString(l.Source)
buf.WriteRune(':')
buf.WriteString(l.Key)
buf.WriteRune('=')
buf.WriteString(l.Value)
buf.WriteRune(';')
return buf.Bytes()
}

// SortedList returns the labels as a sorted list, separated by semicolon
Expand All @@ -500,10 +511,21 @@ func (l Labels) SortedList() []byte {
}
slices.Sort(keys)

b := make([]byte, 0, len(keys)*2)
// Labels can have arbitrary size. However, when many CIDR identities are in
// the system, for example due to a FQDN policy matching S3, CIDR labels
// dominate in number. IPv4 CIDR labels in serialized form are max 25 bytes
// long. Allocate slightly more to avoid having a realloc if there's some
// other labels which may longer, since the cost of allocating a few bytes
// more is dominated by a second allocation, especially since these
// allocations are short-lived.
//
// cidr:123.123.123.123/32=;
// 0 1 2
// 1234567890123456789012345
b := make([]byte, 0, len(keys)*30)
buf := bytes.NewBuffer(b)
for _, k := range keys {
buf.Write(l[k].FormatForKVStore())
l[k].formatForKVStoreInto(buf)
}

return buf.Bytes()
Expand Down

0 comments on commit f41d675

Please sign in to comment.