ROX-31012: Optimize memory for UpdateComputer endpoint dedupers#17040
ROX-31012: Optimize memory for UpdateComputer endpoint dedupers#17040
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
Skipping CI for Draft Pull Request. |
|
Images are ready for the commit at 175c010. To use with deploy scripts, first |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #17040 +/- ##
==========================================
- Coverage 48.86% 48.85% -0.02%
==========================================
Files 2720 2720
Lines 203364 203304 -60
==========================================
- Hits 99374 99319 -55
+ Misses 96166 96163 -3
+ Partials 7824 7822 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0264519 to
26fee46
Compare
f636ddd to
425a08d
Compare
5e67e12 to
677d6ce
Compare
|
/retest |
This comment was marked as off-topic.
This comment was marked as off-topic.
677d6ce to
175c010
Compare
|
/retest |
bf181fd to
41d6369
Compare
janisz
left a comment
There was a problem hiding this comment.
LGTM there are potential perf optimisations but that could be handled in next PR
I added @johannes94 the hashing expert to find other optimisations
|
TBH I don't feel like a hashing expert and it is hard for me to come up with good suggestions without any data to see where allocation and heap usage hot spots are. If I were to find more optimizations I would:
|
|
Naturally! I compared the 41d6369 (Old) against 0c2abb2 (New) Here are the results: Here is the benchmark codepackage indicator
import (
"hash"
"testing"
"github.com/cespare/xxhash"
"github.com/stackrox/rox/generated/storage"
"github.com/stackrox/rox/pkg/networkgraph"
)
// Old implementations (before optimization) for comparison
func hashPortAndProtocolOld(h hash.Hash64, port uint16, protocol storage.L4Protocol) {
// OLD: Two separate allocations
portBytes := [2]byte{byte(port >> 8), byte(port)}
_, _ = h.Write(portBytes[:])
protocolBytes := [4]byte{
byte(protocol >> 24), byte(protocol >> 16),
byte(protocol >> 8), byte(protocol),
}
_, _ = h.Write(protocolBytes[:])
}
func hashStringsOld(h hash.Hash64, strs ...string) {
// OLD: Allocates []byte for delimiter and for each string conversion
for i, s := range strs {
if i > 0 {
_, _ = h.Write([]byte{0}) // Allocation per iteration
}
_, _ = h.Write([]byte(s)) // Allocation per string
}
}
func keyHashOld(srcID, dstID string, port uint16, protocol storage.L4Protocol) string {
h := xxhash.New()
hashStringsOld(h, srcID, dstID)
hashPortAndProtocolOld(h, port, protocol)
return hashToHexString(h.Sum64())
}
// New implementations (current optimized versions) - using the actual functions
func keyHashNew(srcID, dstID string, port uint16, protocol storage.L4Protocol) string {
h := xxhash.New()
hashStrings(h, srcID, dstID)
hashPortAndProtocol(h, port, protocol)
return hashToHexString(h.Sum64())
}
// Benchmark NetworkConn key generation
func BenchmarkNetworkConn_KeyGeneration_Old(b *testing.B) {
entity1 := networkgraph.Entity{Type: storage.NetworkEntityInfo_DEPLOYMENT, ID: "deployment-1-with-a-long-name"}
entity2 := networkgraph.Entity{Type: storage.NetworkEntityInfo_DEPLOYMENT, ID: "deployment-2-with-another-long-name"}
port := uint16(8080)
protocol := storage.L4Protocol_L4_PROTOCOL_TCP
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_ = keyHashOld(entity1.ID, entity2.ID, port, protocol)
}
}
func BenchmarkNetworkConn_KeyGeneration_New(b *testing.B) {
entity1 := networkgraph.Entity{Type: storage.NetworkEntityInfo_DEPLOYMENT, ID: "deployment-1-with-a-long-name"}
entity2 := networkgraph.Entity{Type: storage.NetworkEntityInfo_DEPLOYMENT, ID: "deployment-2-with-another-long-name"}
port := uint16(8080)
protocol := storage.L4Protocol_L4_PROTOCOL_TCP
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_ = keyHashNew(entity1.ID, entity2.ID, port, protocol)
}
}
// Benchmark with real NetworkConn objects
func BenchmarkNetworkConn_Key_Old(b *testing.B) {
entity1 := networkgraph.Entity{Type: storage.NetworkEntityInfo_DEPLOYMENT, ID: "deployment-1-with-a-long-name"}
entity2 := networkgraph.Entity{Type: storage.NetworkEntityInfo_DEPLOYMENT, ID: "deployment-2-with-another-long-name"}
conn := NetworkConn{
SrcEntity: entity1,
DstEntity: entity2,
DstPort: 8080,
Protocol: storage.L4Protocol_L4_PROTOCOL_TCP,
}
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_ = keyHashOld(conn.SrcEntity.ID, conn.DstEntity.ID, conn.DstPort, conn.Protocol)
}
}
func BenchmarkNetworkConn_Key_New(b *testing.B) {
entity1 := networkgraph.Entity{Type: storage.NetworkEntityInfo_DEPLOYMENT, ID: "deployment-1-with-a-long-name"}
entity2 := networkgraph.Entity{Type: storage.NetworkEntityInfo_DEPLOYMENT, ID: "deployment-2-with-another-long-name"}
conn := NetworkConn{
SrcEntity: entity1,
DstEntity: entity2,
DstPort: 8080,
Protocol: storage.L4Protocol_L4_PROTOCOL_TCP,
}
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_ = conn.Key()
}
}
// Benchmark ContainerEndpoint key generation
func BenchmarkContainerEndpoint_Old(b *testing.B) {
entity := networkgraph.Entity{Type: storage.NetworkEntityInfo_DEPLOYMENT, ID: "deployment-with-a-very-long-identifier"}
port := uint16(443)
protocol := storage.L4Protocol_L4_PROTOCOL_TCP
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_ = keyHashOld(entity.ID, "", port, protocol)
}
}
func BenchmarkContainerEndpoint_New(b *testing.B) {
entity := networkgraph.Entity{Type: storage.NetworkEntityInfo_DEPLOYMENT, ID: "deployment-with-a-very-long-identifier"}
endpoint := ContainerEndpoint{
Entity: entity,
Port: 443,
Protocol: storage.L4Protocol_L4_PROTOCOL_TCP,
}
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_ = endpoint.BinaryKey()
}
}
// Benchmark ProcessListening key generation
func BenchmarkProcessListening_Old(b *testing.B) {
podID := "nginx-pod-12345678-abcd"
containerName := "nginx-container-with-long-name"
processName := "nginx"
processExec := "/usr/sbin/nginx"
processArgs := "-g daemon off; -c /etc/nginx/nginx.conf"
port := uint16(80)
protocol := storage.L4Protocol_L4_PROTOCOL_TCP
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
h := xxhash.New()
hashStringsOld(h, podID, containerName, processName, processExec, processArgs)
hashPortAndProtocolOld(h, port, protocol)
_ = BinaryHash(h.Sum64())
}
}
func BenchmarkProcessListening_New(b *testing.B) {
process := ProcessListening{
Process: ProcessInfo{
ProcessName: "nginx",
ProcessArgs: "-g daemon off; -c /etc/nginx/nginx.conf",
ProcessExec: "/usr/sbin/nginx",
},
PodID: "nginx-pod-12345678-abcd",
ContainerName: "nginx-container-with-long-name",
DeploymentID: "nginx-deployment",
PodUID: "pod-uid-456",
Namespace: "default",
Port: 80,
Protocol: storage.L4Protocol_L4_PROTOCOL_TCP,
}
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_ = process.BinaryKey()
}
}
// Benchmark individual optimization components
func BenchmarkHashPortAndProtocol_Old(b *testing.B) {
h := xxhash.New()
port := uint16(8080)
protocol := storage.L4Protocol_L4_PROTOCOL_TCP
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
hashPortAndProtocolOld(h, port, protocol)
h.Reset()
}
}
func BenchmarkHashPortAndProtocol_New(b *testing.B) {
h := xxhash.New()
port := uint16(8080)
protocol := storage.L4Protocol_L4_PROTOCOL_TCP
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
hashPortAndProtocol(h, port, protocol)
h.Reset()
}
}
func BenchmarkHashStrings_Old(b *testing.B) {
h := xxhash.New()
str1 := "deployment-1-with-a-long-name"
str2 := "deployment-2-with-another-long-name"
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
hashStringsOld(h, str1, str2)
h.Reset()
}
}
func BenchmarkHashStrings_New(b *testing.B) {
h := xxhash.New()
str1 := "deployment-1-with-a-long-name"
str2 := "deployment-2-with-another-long-name"
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
hashStrings(h, str1, str2)
h.Reset()
}
}
// Benchmark with varying string counts
func BenchmarkHashStrings_Old_5Strings(b *testing.B) {
h := xxhash.New()
strs := []string{
"nginx-pod-12345678-abcd",
"nginx-container-with-long-name",
"nginx",
"/usr/sbin/nginx",
"-g daemon off; -c /etc/nginx/nginx.conf",
}
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
hashStringsOld(h, strs...)
h.Reset()
}
}
func BenchmarkHashStrings_New_5Strings(b *testing.B) {
h := xxhash.New()
strs := []string{
"nginx-pod-12345678-abcd",
"nginx-container-with-long-name",
"nginx",
"/usr/sbin/nginx",
"-g daemon off; -c /etc/nginx/nginx.conf",
}
b.ResetTimer()
b.ReportAllocs()
for i := 0; i < b.N; i++ {
hashStrings(h, strs...)
h.Reset()
}
} |
|
/retest |
|
/retest |
1 similar comment
|
/retest |
|
@vikin91: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
(OPTIONAL FOR 4.9)
Description
This PR changes the key-value type of endpoints deduper to use
[8]byteuint64instead ofstring. That significantly reduces the memory footprint of the deduper (in theory by 50%) without loosing accuracy. The 8-byte type is used because of the 64-bit hashing with collision probability of 2.7% for 100M entries*.Drawbacks:
*) Long running cluster running with 100 open endpoints per second generates roughly 16M entries in 3 days.
Increasing the hash length to 128-bit lowers the collision chance to 1 in ~10⁻²⁰ while using the same amount of memory as the current
map[string]stringimplementation.User-facing documentation
Testing and quality
Automated testing
How I validated my change
Insights into deduper memory after 2 days of long-running cluster with fake workloads
Size of single deduper entry
piotr/ROX-30941-no-deduping-processes)