maglev: Cleanup implementation #35885
Merged
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
|
/test |
tommyp1ckles
approved these changes
Nov 13, 2024
8 tasks
ldelossa
approved these changes
Nov 18, 2024
31006dd to
8d30867
Compare
8d30867 to
5b9ccd7
Compare
jrajahalme
approved these changes
Nov 29, 2024
cb20112 to
7f7a5b5
Compare
|
/test |
7f7a5b5 to
1b85500
Compare
|
/test |
1b85500 to
58cc253
Compare
|
/test |
39bebc2 to
9286446
Compare
|
/test |
6846835 to
a6f67d1
Compare
|
/test |
Benchmark also the more realistic smaller sizes. Results from my machine: goos: linux goarch: amd64 pkg: github.com/cilium/cilium/pkg/maglev cpu: 13th Gen Intel(R) Core(TM) i9-13950HX BenchmarkGetMaglevTable/2039-32 681 1746071 ns/op 130665 B/op 151 allocs/op BenchmarkGetMaglevTable/4093-32 367 2793396 ns/op 212250 B/op 151 allocs/op BenchmarkGetMaglevTable/16381-32 135 8171612 ns/op 1192045 B/op 151 allocs/op BenchmarkGetMaglevTable/131071-32 22 52912588 ns/op 1138965 B/op 151 allocs/op Signed-off-by: Jussi Maki <jussi@isovalent.com>
Add a test to maglev to make sure the generated table is always the same with the default configuration. This ensures we don't accidentally change the algorithm and end up with differing maglev tables on nodes when upgrading. Signed-off-by: Jussi Maki <jussi@isovalent.com>
Parallel test cases in pkg/loadbalancer/experimental are causing racy access to the maglev permutations array. Refactor the maglev package into a cell with its own configuration to allow safely using it in parallel tests with potentially different configurations. To avoid needless copying and construction of temporary data structures, pass the backends as a sequence to GetLookupTable. Benchmark results: goos: linux goarch: amd64 pkg: github.com/cilium/cilium/pkg/maglev cpu: 13th Gen Intel(R) Core(TM) i9-13950HX BenchmarkGetMaglevTable/2039-32 724 1668577 ns/op 55210 B/op 146 allocs/op BenchmarkGetMaglevTable/4093-32 456 2624453 ns/op 112473 B/op 146 allocs/op BenchmarkGetMaglevTable/16381-32 135 8564586 ns/op 1060573 B/op 146 allocs/op BenchmarkGetMaglevTable/131071-32 25 49359121 ns/op 548535 B/op 146 allocs/op Signed-off-by: Jussi Maki <jussi@isovalent.com>
a6f67d1 to
2993089
Compare
|
/test |
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.
This makes the maglev implementation a bit more generic by not depending on the
loadbalancer.Backendtype directly, but instead just taking in the bare minimum. To avoid temporary data structures the backend infos are passed as a iter.Seq. This allows #35430 to avoid the sillyness of convertingexperimental.Backendintoloadbalancer.Backendand having to build up temporary hash maps.To make sure the refactoring does not introduce any changes the
TestReproduciblehard-codes the expected maglev table to check against.