Improve the performance of endpoints correlation in service cache #35604
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In preparation for the subsequent changes, let's introduce a dedicated benchmark for correlateEndpoints, so that we can measure the improvements. Specifically, the benchmark initializes a service associated with 10 epslices, each including 100 endpoints (2 ports each), and then repeatedly calls correlateEndpoints measuring CPU time and memory allocations. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Reduce the number of allocations (and consequently CPU time) performed
by the correlateEndpoints function removing a duplicated DeepCopy of
all endpoints. Indeed, GetEndpoints already returns a DeepCopy, and we
can subsequently mutate the endpoint information in-place, without
needing to copy them again. Additionally, let's rework a bit the logic
to avoid unnecessarily creating a second map and copy all entries there.
Considering BenchmarkCorrelateEndpoints which calls correlateEndpoints
for a service associated with 10 epslices, each with 100 endpoints (2
ports each), these changes lead to approximately a 50% CPU and memory
allocations reduction:
goos: linux
goarch: amd64
pkg: github.com/cilium/cilium/pkg/k8s
cpu: 13th Gen Intel(R) Core(TM) i7-13800H
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
CorrelateEndpoints-20 1140.8µ ± 4% 605.0µ ± 8% -46.96% (p=0.000 n=10)
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
CorrelateEndpoints-20 1177.9Ki ± 0% 589.0Ki ± 0% -50.00% (p=0.000 n=10)
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
CorrelateEndpoints-20 10.062k ± 0% 5.031k ± 0% -50.00% (p=0.000 n=10)
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
|
/test |
|
Apparently kicked out of the merge queue due to the ongoing GitHub Actions incident: https://www.githubstatus.com/ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reduce the number of allocations (and consequently CPU time) performed by the
correlateEndpointsfunction removing a duplicated DeepCopy of all endpoints. Indeed, GetEndpoints already returns a DeepCopy, and we can subsequently mutate the endpoint information in-place, without needing to copy them again. Additionally, let's rework a bit the logicto avoid unnecessarily creating a second map and copy all entries there.
Considering
BenchmarkCorrelateEndpointswhich callscorrelateEndpointsfor a service associated with 10 epslices, each with 100 endpoints (2 ports each), these changes lead to approximately a 50% CPU and memory allocations reduction: