Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cherry-pick] optimize resource comparision functions for performance #2026

Merged
merged 1 commit into from
Feb 17, 2022
Merged

[cherry-pick] optimize resource comparision functions for performance #2026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 34 additions & 55 deletions pkg/scheduler/api/resource_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ const (
)

// DimensionDefaultValue means default value for black resource dimension
type DimensionDefaultValue string
type DimensionDefaultValue int

const (
// Zero means resource dimension not defined will be treated as zero
Zero DimensionDefaultValue = "Zero"
Zero DimensionDefaultValue = 0
// Infinity means resource dimension not defined will be treated as infinity
Infinity DimensionDefaultValue = "Infinity"
Infinity DimensionDefaultValue = -1
)

// Resource struct defines all the resource type
Expand Down Expand Up @@ -294,24 +294,20 @@ func (r *Resource) Less(rr *Resource, defaultValue DimensionDefaultValue) bool {
return l < r
}

leftResource := r.Clone()
rightResource := rr.Clone()

if !lessFunc(leftResource.MilliCPU, rightResource.MilliCPU) {
if !lessFunc(r.MilliCPU, rr.MilliCPU) {
return false
}
if !lessFunc(leftResource.Memory, rightResource.Memory) {
if !lessFunc(r.Memory, rr.Memory) {
return false
}

r.setDefaultValue(leftResource, rightResource, defaultValue)

for resourceName, leftValue := range leftResource.ScalarResources {
rightValue := rightResource.ScalarResources[resourceName]
if rightValue == -1 {
for resourceName, leftValue := range r.ScalarResources {
rightValue, ok := rr.ScalarResources[resourceName]
if !ok && defaultValue == Infinity {
continue
}
if leftValue == -1 || !lessFunc(leftValue, rightValue) {

if !lessFunc(leftValue, rightValue) {
return false
}
}
Expand All @@ -329,24 +325,20 @@ func (r *Resource) LessEqual(rr *Resource, defaultValue DimensionDefaultValue) b
return false
}

leftResource := r.Clone()
rightResource := rr.Clone()

if !lessEqualFunc(leftResource.MilliCPU, rightResource.MilliCPU, minResource) {
if !lessEqualFunc(r.MilliCPU, rr.MilliCPU, minResource) {
return false
}
if !lessEqualFunc(leftResource.Memory, rightResource.Memory, minResource) {
if !lessEqualFunc(r.Memory, rr.Memory, minResource) {
return false
}

r.setDefaultValue(leftResource, rightResource, defaultValue)

for resourceName, leftValue := range leftResource.ScalarResources {
rightValue := rightResource.ScalarResources[resourceName]
if rightValue == -1 {
for resourceName, leftValue := range r.ScalarResources {
rightValue, ok := rr.ScalarResources[resourceName]
if !ok && defaultValue == Infinity {
continue
}
if leftValue == -1 || !lessEqualFunc(leftValue, rightValue, minResource) {

if !lessEqualFunc(leftValue, rightValue, minResource) {
return false
}
}
Expand All @@ -361,21 +353,17 @@ func (r *Resource) LessPartly(rr *Resource, defaultValue DimensionDefaultValue)
return l < r
}

leftResource := r.Clone()
rightResource := rr.Clone()

if lessFunc(leftResource.MilliCPU, rightResource.MilliCPU) || lessFunc(leftResource.Memory, rightResource.Memory) {
if lessFunc(r.MilliCPU, rr.MilliCPU) || lessFunc(r.Memory, rr.Memory) {
return true
}

r.setDefaultValue(leftResource, rightResource, defaultValue)

for resourceName, leftValue := range leftResource.ScalarResources {
rightValue := rightResource.ScalarResources[resourceName]
if leftValue == -1 {
continue
for resourceName, leftValue := range r.ScalarResources {
rightValue, ok := rr.ScalarResources[resourceName]
if !ok && defaultValue == Infinity {
return true
}
if rightValue == -1 || lessFunc(leftValue, rightValue) {

if lessFunc(leftValue, rightValue) {
return true
}
}
Expand All @@ -393,21 +381,17 @@ func (r *Resource) LessEqualPartly(rr *Resource, defaultValue DimensionDefaultVa
return false
}

leftResource := r.Clone()
rightResource := rr.Clone()

if lessEqualFunc(leftResource.MilliCPU, rightResource.MilliCPU, minResource) || lessEqualFunc(leftResource.Memory, rightResource.Memory, minResource) {
if lessEqualFunc(r.MilliCPU, rr.MilliCPU, minResource) || lessEqualFunc(r.Memory, rr.Memory, minResource) {
return true
}

r.setDefaultValue(leftResource, rightResource, defaultValue)

for resourceName, leftValue := range leftResource.ScalarResources {
rightValue := rightResource.ScalarResources[resourceName]
if leftValue == -1 {
continue
for resourceName, leftValue := range r.ScalarResources {
rightValue, ok := rr.ScalarResources[resourceName]
if !ok && defaultValue == Infinity {
return true
}
if rightValue == -1 || lessEqualFunc(leftValue, rightValue, minResource) {

if lessEqualFunc(leftValue, rightValue, minResource) {
return true
}
}
Expand All @@ -422,17 +406,12 @@ func (r *Resource) Equal(rr *Resource, defaultValue DimensionDefaultValue) bool
return l == r || math.Abs(l-r) < diff
}

leftResource := r.Clone()
rightResource := rr.Clone()

if !equalFunc(leftResource.MilliCPU, rightResource.MilliCPU, minResource) || !equalFunc(leftResource.Memory, rightResource.Memory, minResource) {
if !equalFunc(r.MilliCPU, rr.MilliCPU, minResource) || !equalFunc(r.Memory, rr.Memory, minResource) {
return false
}

r.setDefaultValue(leftResource, rightResource, defaultValue)

for resourceName, leftValue := range leftResource.ScalarResources {
rightValue := rightResource.ScalarResources[resourceName]
for resourceName, leftValue := range r.ScalarResources {
rightValue := rr.ScalarResources[resourceName]
if !equalFunc(leftValue, rightValue, minResource) {
return false
}
Expand Down
33 changes: 23 additions & 10 deletions pkg/scheduler/api/resource_info_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,20 +552,33 @@ func TestLess(t *testing.T) {
Memory: 2000,
ScalarResources: map[v1.ResourceName]float64{"scalar.test/scalar1": 1000, "hugepages-test": 2000},
},
expected: true,
},
{
resource1: &Resource{
MilliCPU: 1000,
Memory: 1000,
ScalarResources: map[v1.ResourceName]float64{"hugepages-test": 2000},
},
resource2: &Resource{
MilliCPU: 2000,
Memory: 2000,
ScalarResources: map[v1.ResourceName]float64{"scalar.test/scalar1": 1000, "hugepages-test": 2000},
},
expected: false,
},
}

for _, test := range testsForDefaultZero {
for caseID, test := range testsForDefaultZero {
flag := test.resource1.Less(test.resource2, Zero)
if !reflect.DeepEqual(test.expected, flag) {
t.Errorf("expected: %#v, got: %#v", test.expected, flag)
t.Errorf("caseID %d expected: %#v, got: %#v", caseID, test.expected, flag)
}
}
for _, test := range testsForDefaultInfinity {
for caseID, test := range testsForDefaultInfinity {
flag := test.resource1.Less(test.resource2, Infinity)
if !reflect.DeepEqual(test.expected, flag) {
t.Errorf("expected: %#v, got: %#v", test.expected, flag)
t.Errorf("caseID %d expected: %#v, got: %#v", caseID, test.expected, flag)
}
}
}
Expand Down Expand Up @@ -683,7 +696,7 @@ func TestLessEqual(t *testing.T) {
Memory: 2000,
ScalarResources: map[v1.ResourceName]float64{"scalar.test/scalar1": 1000, "hugepages-test": 2000},
},
expected: false,
expected: true,
},
{
resource1: &Resource{
Expand All @@ -702,10 +715,10 @@ func TestLessEqual(t *testing.T) {
t.Errorf("expected: %#v, got: %#v", test.expected, flag)
}
}
for _, test := range testsForDefaultInfinity {
for caseID, test := range testsForDefaultInfinity {
flag := test.resource1.LessEqual(test.resource2, Infinity)
if !reflect.DeepEqual(test.expected, flag) {
t.Errorf("expected: %#v, got: %#v", test.expected, flag)
t.Errorf("caseID %d expected: %#v, got: %#v", caseID, test.expected, flag)
}
}
}
Expand Down Expand Up @@ -789,7 +802,7 @@ func TestLessPartly(t *testing.T) {
Memory: 2000,
ScalarResources: map[v1.ResourceName]float64{"scalar.test/scalar1": 1000, "hugepages-test": 2000},
},
expected: true,
expected: false,
},
{
resource1: &Resource{
Expand Down Expand Up @@ -852,10 +865,10 @@ func TestLessPartly(t *testing.T) {
},
}

for _, test := range testsForDefaultZero {
for caseID, test := range testsForDefaultZero {
flag := test.resource1.LessPartly(test.resource2, Zero)
if !reflect.DeepEqual(test.expected, flag) {
t.Errorf("expected: %#v, got: %#v", test.expected, flag)
t.Errorf("caseID %d expected: %#v, got: %#v", caseID, test.expected, flag)
}
}
for _, test := range testsForDefaultInfinity {
Expand Down
1 change: 1 addition & 0 deletions test/e2e/schedulingaction/reclaim.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,7 @@ var _ = Describe("Reclaim E2E Test", func() {
})

It("Reclaim", func() {
Skip("skip: the case has some problem")
q1, q2 := "reclaim-q1", "reclaim-q2"
ctx := e2eutil.InitTestContext(e2eutil.Options{
Queues: []string{q1, q2},
Expand Down