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.

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

api/resource: optimize scale function #18170

Merged
merged 1 commit into from
Dec 9, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 2 additions & 4 deletions pkg/api/resource/quantity.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,7 @@ func (q *Quantity) Value() int64 {
if q.Amount == nil {
return 0
}
tmp := &inf.Dec{}
return tmp.Round(q.Amount, 0, inf.RoundUp).UnscaledBig().Int64()
return scaledValue(q.Amount.UnscaledBig(), int(q.Amount.Scale()), 0)
}

// MilliValue returns the value of q * 1000; this could overflow an int64;
Expand All @@ -387,8 +386,7 @@ func (q *Quantity) MilliValue() int64 {
if q.Amount == nil {
return 0
}
tmp := &inf.Dec{}
return tmp.Round(tmp.Mul(q.Amount, decThousand), 0, inf.RoundUp).UnscaledBig().Int64()
return scaledValue(q.Amount.UnscaledBig(), int(q.Amount.Scale()), 3)
}

// Set sets q's value to be value.
Expand Down
95 changes: 95 additions & 0 deletions pkg/api/resource/scale_int.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
Copyright 2015 The Kubernetes Authors All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package resource

import (
"math"
"math/big"
"sync"
)

var (
// A sync pool to reduce allocation.
intPool sync.Pool
maxInt64 = big.NewInt(math.MaxInt64)
)

func init() {
intPool.New = func() interface{} {
return &big.Int{}
}
}

// scaledValue scales given unscaled value from scale to new Scale and returns
// an int64. It ALWAYS rounds up the result when scale down. The final result might
// overflow.
//
// scale, newScale represents the scale of the unscaled decimal.
// The mathematical value of the decimal is unscaled * 10**(-scale).
func scaledValue(unscaled *big.Int, scale, newScale int) int64 {
dif := scale - newScale
if dif == 0 {
return unscaled.Int64()
}

// Handle scale up
// This is an easy case, we do not need to care about rounding and overflow.
// If any intermediate operation causes overflow, the result will overflow.
if dif < 0 {
return unscaled.Int64() * int64(math.Pow10(-dif))
}

// Handle scale down
// We have to be careful about the intermediate operations.

// fast path when unscaled < max.Int64 and exp(10,dif) < max.Int64
const log10MaxInt64 = 19
if unscaled.Cmp(maxInt64) < 0 && dif < log10MaxInt64 {
divide := int64(math.Pow10(dif))
result := unscaled.Int64() / divide
mod := unscaled.Int64() % divide
if mod != 0 {
return result + 1
}
return result
}

// We should only convert back to int64 when getting the result.
divisor := intPool.Get().(*big.Int)
exp := intPool.Get().(*big.Int)
result := intPool.Get().(*big.Int)
defer func() {
intPool.Put(divisor)
intPool.Put(exp)
intPool.Put(result)
}()

// divisor = 10^(dif)
// TODO: create loop up table if exp costs too much.
divisor.Exp(bigTen, exp.SetInt64(int64(dif)), nil)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(optional nit) you could use divisor here in place of exp, then you don't need remainder := exp below. That might be a little cleaner since it sets it to the final value in one line... up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel I still want to keep them separate.

// reuse exp
remainder := exp

// result = unscaled / divisor
// remainder = unscaled % divisor
result.DivMod(unscaled, divisor, remainder)
if remainder.Sign() != 0 {
return result.Int64() + 1
}

return result.Int64()
}
85 changes: 85 additions & 0 deletions pkg/api/resource/scale_int_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
Copyright 2015 The Kubernetes Authors All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package resource

import (
"math"
"math/big"
"testing"
)

func TestScaledValue(t *testing.T) {
tests := []struct {
unscaled *big.Int
scale int
newScale int

want int64
}{
// remain scale
{big.NewInt(1000), 0, 0, 1000},

// scale down

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see more test cases in each group: scales are zero, unscaled is negative, small, big, zero, etc.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, check some values around max Int64 with high precision to catch the case where you're losing precision.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added.

{big.NewInt(1000), 0, -3, 1},
{big.NewInt(1000), 3, 0, 1},
{big.NewInt(0), 3, 0, 0},

// always round up
{big.NewInt(999), 3, 0, 1},
{big.NewInt(500), 3, 0, 1},
{big.NewInt(499), 3, 0, 1},
{big.NewInt(1), 3, 0, 1},
// large scaled value does not lose precision
{big.NewInt(0).Sub(maxInt64, bigOne), 1, 0, (math.MaxInt64-1)/10 + 1},
// large intermidiate result.
{big.NewInt(1).Exp(big.NewInt(10), big.NewInt(100), nil), 100, 0, 1},

// scale up
{big.NewInt(0), 0, 3, 0},
{big.NewInt(1), 0, 3, 1000},
{big.NewInt(1), -3, 0, 1000},
{big.NewInt(1000), -3, 2, 100000000},
{big.NewInt(0).Div(big.NewInt(math.MaxInt64), bigThousand), 0, 3,
(math.MaxInt64 / 1000) * 1000},
}

for i, tt := range tests {
old := (&big.Int{}).Set(tt.unscaled)
got := scaledValue(tt.unscaled, tt.scale, tt.newScale)
if got != tt.want {
t.Errorf("#%d: got = %v, want %v", i, got, tt.want)
}
if tt.unscaled.Cmp(old) != 0 {
t.Errorf("#%d: unscaled = %v, want %v", i, tt.unscaled, old)
}
}
}

func BenchmarkScaledValueSmall(b *testing.B) {
s := big.NewInt(1000)
for i := 0; i < b.N; i++ {
scaledValue(s, 3, 0)
}
}

func BenchmarkScaledValueLarge(b *testing.B) {
s := big.NewInt(math.MaxInt64)
s.Mul(s, big.NewInt(1000))
for i := 0; i < b.N; i++ {
scaledValue(s, 10, 0)
}
}