improve the nodeclaim sorting algorithm using heap #1844
Conversation
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jxs1211 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
cncf-cla: no
|
Welcome @jxs1211! |
|
Hi @jxs1211. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions 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. |
k8s-ci-robot
added
needs-ok-to-test
Pull Request Test Coverage Report for Build 12022204027Details
💛 - Coveralls |
|
This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity. |
github-actions
bot
added
the
lifecycle/stale
| // Consider using https://pkg.go.dev/container/heap | ||
| sort.Slice(s.newNodeClaims, func(a, b int) bool { return len(s.newNodeClaims[a].Pods) < len(s.newNodeClaims[b].Pods) }) | ||
|
|
||
| // Pick existing node that we are about to create | ||
| for _, nodeClaim := range s.newNodeClaims { | ||
| h := NewNodeClaimHeap(s.newNodeClaims) | ||
| heap.Init(h) | ||
| for h.Len() > 0 { | ||
| nodeClaim := heap.Pop(h).(*NodeClaim) |
There was a problem hiding this comment.
We should write some benchmarking tests to validate this makes things faster and by how much.
There was a problem hiding this comment.
I add some benchmark for evaluation, basically heap is better but not that much.
➜ karpenter git:(improve/nodeclaim-sorting) ✗ go test -run=none -bench=Sorting -benchmem sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling -v
goos: linux
goarch: amd64
pkg: sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling
cpu: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
BenchmarkHeapSorting
BenchmarkHeapSorting-4 19477929 86.49 ns/op 24 B/op 1 allocs/op
BenchmarkSliceSorting
BenchmarkSliceSorting-4 8984490 116.3 ns/op 24 B/op 1 allocs/op
PASS
ok sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling 4.877s
➜ karpenter git:(improve/nodeclaim-sorting) ✗
➜ karpenter git:(improve/nodeclaim-sorting) ✗ go test -run=none -bench=Sorting -benchmem sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling -v
goos: linux
goarch: amd64
pkg: sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling
cpu: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
BenchmarkHeapSorting
BenchmarkHeapSorting-4 16447299 62.18 ns/op 24 B/op 1 allocs/op
BenchmarkSliceSorting
BenchmarkSliceSorting-4 13348653 78.73 ns/op 24 B/op 1 allocs/op
PASS
ok sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling 2.304s
➜ karpenter git:(improve/nodeclaim-sorting) ✗ go test -run=none -bench=Sorting -benchmem sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling -v
goos: linux
goarch: amd64
pkg: sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling
cpu: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
BenchmarkHeapSorting
BenchmarkHeapSorting-4 17975103 58.27 ns/op 24 B/op 1 allocs/op
BenchmarkSliceSorting
BenchmarkSliceSorting-4 13778595 88.28 ns/op 24 B/op 1 allocs/op
PASS
ok sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling 2.478s
➜ karpenter git:(improve/nodeclaim-sorting) ✗ go test -run=none -bench=Sorting -benchmem sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling -v
goos: linux
goarch: amd64
pkg: sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling
cpu: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
BenchmarkHeapSorting
BenchmarkHeapSorting-4 20438842 59.15 ns/op 24 B/op 1 allocs/op
BenchmarkSliceSorting
BenchmarkSliceSorting-4 15978789 75.94 ns/op 24 B/op 1 allocs/op
PASS
ok sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling 6.341s
➜ karpenter git:(improve/nodeclaim-sorting) ✗ go test -run=none -bench=Sorting -benchmem sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling -v
goos: linux
goarch: amd64
pkg: sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling
cpu: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
BenchmarkHeapSorting
BenchmarkHeapSorting-4 17908522 71.79 ns/op 24 B/op 1 allocs/op
BenchmarkSliceSorting
BenchmarkSliceSorting-4 13740463 77.62 ns/op 24 B/op 1 allocs/op
PASS
ok sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling 2.566s
➜ karpenter git:(improve/nodeclaim-sorting) ✗ go test -run=none -bench=Sorting -benchmem sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling -v
goos: linux
goarch: amd64
pkg: sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling
cpu: Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
BenchmarkHeapSorting
BenchmarkHeapSorting-4 16799110 67.39 ns/op 24 B/op 1 allocs/op
BenchmarkSliceSorting
BenchmarkSliceSorting-4 13493124 97.02 ns/op 24 B/op 1 allocs/op
PASS
ok sigs.k8s.io/karpenter/pkg/controllers/provisioning/scheduling 4.522s
➜ karpenter git:(improve/nodeclaim-sorting) ✗
github-actions
bot
removed
the
lifecycle/stale
|
This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity. |
github-actions
bot
added
the
lifecycle/stale
jxs1211
force-pushed
the
improve/nodeclaim-sorting
branch
from
998a3fe to
7e40621
Compare
Signed-off-by: Jay Shane <[email protected]>
jxs1211
force-pushed
the
improve/nodeclaim-sorting
branch
from
7e40621 to
8b2ca03
Compare
github-actions
bot
removed
the
lifecycle/stale
perf
Description
improve the nodeclaim sorting according to the todo comment
How was this change tested?
implementation with test case
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.