From 6c6cdac0e9875b60578e635984d6a4f42cba97dd Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Fri, 2 Jan 2015 13:42:27 -0800 Subject: [PATCH] Add accessor functions; round-trip test --- pkg/api/resource/quantity.go | 254 +++++++++++++++++++++--------- pkg/api/resource/quantity_test.go | 183 +++++++++++++++++++-- pkg/api/resource/suffix.go | 2 +- 3 files changed, 348 insertions(+), 91 deletions(-) diff --git a/pkg/api/resource/quantity.go b/pkg/api/resource/quantity.go index 58509547df2..d7e2ab595d3 100644 --- a/pkg/api/resource/quantity.go +++ b/pkg/api/resource/quantity.go @@ -25,15 +25,6 @@ import ( "speter.net/go/exp/math/dec/inf" ) -// Format lists the three possible formattings of a quantity. -type Format string - -const ( - DecimalExponent = Format("DecExponent") - BinarySI = Format("BinSI") - DecimalSI = Format("DecSI") -) - // Quantity is a fixed-point representation of a number. // It provides convenient marshaling/unmarshaling in JSON and YAML, // in addition to String() and Int64() accessors. @@ -41,48 +32,43 @@ const ( // The serialization format is: // // ::= | -// ::= | . +// ::= | . +// (Note that may be ""!) // ::= "+" | "-" // ::= | +// ::= | // ::= 0 | 1 | ... | 9 -// ::= | | +// ::= | | // ::= i | Ki | Mi | Gi | Ti | Pi | Ei // ::= m | "" | k | M | G | T | P | E -// ::= "e" | "E" -// (Where digits is always a multiple of 3) +// ::= "e" | "E" // (Note that 1024 = 1Ki but 1000 = 1k; I didn't choose the capitalization.) // // No matter which of the three exponent forms is used, no quantity may represent -// a number less than 1m or greater than 2^63-1 in magnitude. Numbers that exceed -// a bound will be capped at that bound. (E.g.: 0.1m will be treated as 1m.) +// a number greater than 2^63-1 in magnitude, nor may it have more than 3 digits +// of precision. Numbers larger or more precise will be capped or rounded. +// (E.g.: 0.1m will rounded up to 1m.) // This may be extended in the future if we require larger or smaller quantities. // -// Numbers with binary suffixes may not have any fractional part. +// When a Quantity is parsed from a string, it will remember the type of suffix +// it had, and will use the same type again when it is serialized. +// One exception: numbers with a Binary SI suffix less than one will be changed +// to Decimal SI suffix. E.g., .5i becomes 500m. [NOT 512m!] // -// Quantities will be serialized in the same format that they were parsed from. // Before serializing, Quantity will be put in "canonical form". -// This means that Exponent will be adjusted up or down (with a -// corresponding increase or decrease in Mantissa) until one of the -// following is true: -// a. Binary SI mode: Mantissa mod 1024 is nonzero. -// Examples: 1Gi 300Mi 6Ki 1001Gi -// Non-canonical: 1024Gi (Should be 1Ti) -// b. Decimal SI mode: exponent is greater than 3 and one of Mantissa's three least -// significant digits is nonzero. -// Examples: 1G 300M 3K 1001G -// Non-canonical: 1000G (Should be 1T) -// c. Decimal SI mode: exponent is less than or equal to zero, and Mantissa has no more -// than three nonzero decimals. If any decimals are nonzero, three -// decimals will be emitted. -// Examples: 5 123.450 1.001 0.045 -// Non-canonical: 1m (should be 0.001) -// d. Decimal exponent mode: as for decimal SI mode, but using the corresponding -// "e6" or "E6" form. -// +// This means that Exponent/suffix will be adjusted up or down (with a +// corresponding increase or decrease in Mantissa) such that: +// a. No precision is lost +// b. No fractional digits will be emitted +// c. The exponent (or suffix) is as large as possible. // The sign will be omitted unless the number is negative. // -// Note that the quantity will NEVER be represented by a floating point number. That is -// the whole point of this exercise. +// Examples: +// 1.5 will be serialized as "1500m" +// 1.5Gi will be serialized as "1576Mi" +// +// Note that the quantity will NEVER be internally represented by a +// floating point number. That is the whole point of this exercise. // // Non-canonical values will still parse as long as they are well formed, // but will be re-emitted in their canonical form. (So always use canonical @@ -92,11 +78,28 @@ const ( // writing some sort of special handling code in the hopes that that will // cause implementors to also use a fixed point implementation. type Quantity struct { + // Amount is public, so you can manipulate it if the accessor + // functions are not sufficient. Amount *inf.Dec + + // Change Format at will. See the comment for Canonicalize for + // more details. Format } +// Format lists the three possible formattings of a quantity. +type Format string + const ( + DecimalExponent = Format("DecExponent") + // SI = International System of units. + BinarySI = Format("BinSI") + DecimalSI = Format("DecSI") +) + +const ( + // splitREString is used to separate a number from its suffix; as such, + // this is overly permissive, but that's OK-- it will be checked later. splitREString = "^([+-]?[0123456789.]+)([eEimkKMGTP]*[-+]?[0123456789]*)$" ) @@ -105,21 +108,31 @@ var ( splitRE = regexp.MustCompile(splitREString) // Errors that could happen while parsing a string. - ErrFormatWrong = errors.New("quantities must match the regular expression '" + splitREString + "'") - ErrNumeric = errors.New("unable to parse numeric part of quantity") - ErrSuffix = errors.New("unable to parse quantity's suffix") - ErrFractionalBinary = errors.New("numbers with binary-style SI suffixes can't have fractional parts") + ErrFormatWrong = errors.New("quantities must match the regular expression '" + splitREString + "'") + ErrNumeric = errors.New("unable to parse numeric part of quantity") + ErrSuffix = errors.New("unable to parse quantity's suffix") - // Commonly needed big.Ints-- treat as read only! + // Commonly needed big.Int values-- treat as read only! ten = big.NewInt(10) zero = big.NewInt(0) + one = big.NewInt(1) thousand = big.NewInt(1000) ten24 = big.NewInt(1024) - decZero = inf.NewDec(0, 0) + + // Commonly needed inf.Dec values-- treat as read only! + decZero = inf.NewDec(0, 0) + decOne = inf.NewDec(1, 0) + decMinusOne = inf.NewDec(-1, 0) + decThousand = inf.NewDec(1000, 0) // Smallest and largest (in magnitude) numbers allowed. minAllowed = inf.NewDec(1, 3) // == 1/1000 maxAllowed = inf.NewDec((1<<63)-1, 0) // == max int64 + + // The maximum value we can represent milli-units for. + // Compare with the return value of Quantity.Value() to + // see if it's safe to use Quantity.MilliValue(). + MaxMilliValue = ((1 << 63) - 1) / 1000 ) // ParseQuantity turns str into a Quantity, or returns an error. @@ -143,19 +156,9 @@ func ParseQuantity(str string) (*Quantity, error) { if base == 10 { amount.SetScale(amount.Scale() + inf.Scale(-exponent)) } else if base == 2 { - // Detect fractional parts by rounding. There's probably - // a better way to do this. - if rounded := new(inf.Dec).Round(amount, 0, inf.RoundFloor); rounded.Cmp(amount) != 0 { - return nil, ErrFractionalBinary - } - if exponent < 0 { - return nil, ErrFractionalBinary - } - // exponent will always be a multiple of 10. - for exponent > 0 { - amount.UnscaledBig().Mul(amount.UnscaledBig(), ten24) - exponent -= 10 - } + // numericSuffix = 2 ** exponent + numericSuffix := big.NewInt(1).Lsh(one, uint(exponent)) + amount.UnscaledBig().Mul(amount.UnscaledBig(), numericSuffix) } // Cap at min/max bounds. @@ -175,6 +178,10 @@ func ParseQuantity(str string) (*Quantity, error) { if amount.Cmp(maxAllowed) > 0 { amount.Set(maxAllowed) } + if format == BinarySI && amount.Cmp(decOne) < 0 && amount.Cmp(decZero) > 0 { + // This avoids rounding and hopefully confusion, too. + format = DecimalSI + } if sign == -1 { amount.Neg(amount) } @@ -201,13 +208,45 @@ func removeFactors(d, factor *big.Int) (result *big.Int, times int) { } // Canonicalize returns the canonical form of q and its suffix (see comment on Quantity). +// +// Note about BinarySI: +// * If q.Format is set to BinarySI and q.Amount represents a non-zero value between +// -1 and +1, it will be emitted as if q.Format were DecimalSI. +// * Otherwise, if q.Format is set to BinarySI, frational parts of q.Amount will be +// rounded up. (1.1i becomes 2i.) func (q *Quantity) Canonicalize() (string, suffix) { - mantissa := q.Amount.UnscaledBig() - exponent := int(-q.Amount.Scale()) - amount := big.NewInt(0).Set(mantissa) + if q.Amount == nil { + return "0", "" + } - switch q.Format { + format := q.Format + switch format { case DecimalExponent, DecimalSI: + case BinarySI: + switch q.Amount.Cmp(decZero) { + case 0: // exactly equal 0, that's fine + case 1: // greater than 0 + if q.Amount.Cmp(decOne) < 0 { + // This avoids rounding and hopefully confusion, too. + format = DecimalSI + } + case -1: + if q.Amount.Cmp(decMinusOne) > 0 { + // This avoids rounding and hopefully confusion, too. + format = DecimalSI + } + } + default: + format = DecimalExponent + } + + // TODO: If BinarySI formatting is requested but would cause rounding, upgrade to + // one of the other formats. + switch format { + case DecimalExponent, DecimalSI: + mantissa := q.Amount.UnscaledBig() + exponent := int(-q.Amount.Scale()) + amount := big.NewInt(0).Set(mantissa) // move all factors of 10 into the exponent for easy reasoning amount, times := removeFactors(amount, ten) exponent += times @@ -218,34 +257,24 @@ func (q *Quantity) Canonicalize() (string, suffix) { exponent-- } - absAmount := big.NewInt(0).Abs(amount) - - // Canonical form has three decimal digits. - if absAmount.Cmp(thousand) >= 0 { - // Unless that would cause an exponent of 3-- 111.111e3 is silly. - if exponent != 0 { - suffix, _ := quantitySuffixer.construct(10, exponent+3, q.Format) - number := inf.NewDecBig(amount, 3).String() - return number, suffix - } - } - suffix, _ := quantitySuffixer.construct(10, exponent, q.Format) + suffix, _ := quantitySuffixer.construct(10, exponent, format) number := amount.String() return number, suffix case BinarySI: + tmp := &inf.Dec{} + //tmp.Set(q.Amount) + tmp.Round(q.Amount, 0, inf.RoundUp) + amount := tmp.UnscaledBig() + exponent := int(-q.Amount.Scale()) // Apply the (base-10) shift. This will lose any fractional // part, which is intentional. - for exponent < 0 { - amount.Mul(amount, ten) - exponent++ - } for exponent > 0 { amount.Mul(amount, ten) exponent-- } - amount, exponent := removeFactors(amount, ten24) - suffix, _ := quantitySuffixer.construct(2, exponent*10, q.Format) + amount, exponent = removeFactors(amount, ten24) + suffix, _ := quantitySuffixer.construct(2, exponent*10, format) number := amount.String() return number, suffix } @@ -274,3 +303,72 @@ func (q *Quantity) UnmarshalJSON(value []byte) error { *q = *parsed return nil } + +// NewQuantity returns a new Quantity representing the given +// value in the given format. +func NewQuantity(value int64, format Format) *Quantity { + return &Quantity{ + Amount: inf.NewDec(value, 0), + Format: format, + } +} + +// NewMilliQuantity returns a new Quantity representing the given +// value * 1/1000 in the given format. Note that BinarySI formatting +// will cause rounding for fractional values. +func NewMilliQuantity(value int64, format Format) *Quantity { + return &Quantity{ + Amount: inf.NewDec(value, 3), + Format: format, + } +} + +// Value returns the value of q; any fractional part will be lost. +func (q *Quantity) Value() int64 { + if q.Amount == nil { + return 0 + } + tmp := &inf.Dec{} + return tmp.Round(q.Amount, 0, inf.RoundUp).UnscaledBig().Int64() +} + +// MilliValue returns the value of q * 1000; this could overflow an int64; +// if that's a concern, call Value() first to verify the number is small enough. +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() +} + +// Set sets q's value to be value. +func (q *Quantity) Set(value int64) { + if q.Amount == nil { + q.Amount = &inf.Dec{} + } + q.Amount.SetUnscaled(value) + q.Amount.SetScale(0) +} + +// SetMilli sets q's value to be value * 1/1000. +func (q *Quantity) SetMilli(value int64) { + if q.Amount == nil { + q.Amount = &inf.Dec{} + } + q.Amount.SetUnscaled(value) + q.Amount.SetScale(3) +} + +// Copy is a convenience function that makes a deep copy for you. Non-deep +// copies of quantities share pointers and you will regret that. +func (q *Quantity) Copy() *Quantity { + if q.Amount == nil { + return NewQuantity(0, q.Format) + } + tmp := &inf.Dec{} + return &Quantity{ + Amount: tmp.Set(q.Amount), + Format: q.Format, + } +} diff --git a/pkg/api/resource/quantity_test.go b/pkg/api/resource/quantity_test.go index a94c512ef1e..ee1c1669cc1 100644 --- a/pkg/api/resource/quantity_test.go +++ b/pkg/api/resource/quantity_test.go @@ -18,8 +18,10 @@ package resource import ( //"reflect" + "encoding/json" "testing" + fuzz "github.com/google/gofuzz" "speter.net/go/exp/math/dec/inf" ) @@ -112,7 +114,7 @@ func TestQuantityParse(t *testing.T) { {"10.035M", Quantity{dec(10035, 3), DecimalSI}}, {"1.2e3", Quantity{dec(12, 2), DecimalExponent}}, - {"1.3E6", Quantity{dec(13, 5), DecimalExponent}}, + {"1.3E+6", Quantity{dec(13, 5), DecimalExponent}}, {"1.40e9", Quantity{dec(14, 8), DecimalExponent}}, {"1.53E12", Quantity{dec(153, 10), DecimalExponent}}, {"1.6e15", Quantity{dec(16, 14), DecimalExponent}}, @@ -141,6 +143,21 @@ func TestQuantityParse(t *testing.T) { {"9Ei", Quantity{maxAllowed, BinarySI}}, {"9223372036854775807Ki", Quantity{maxAllowed, BinarySI}}, {"12E", Quantity{maxAllowed, DecimalSI}}, + + // We'll accept fractional binary stuff, too. + {"100.035Ki", Quantity{dec(10243584, -2), BinarySI}}, + {"0.5Mi", Quantity{dec(.5*1024*1024, 0), BinarySI}}, + {"0.05Gi", Quantity{dec(536870912, -1), BinarySI}}, + {"0.025Ti", Quantity{dec(274877906944, -1), BinarySI}}, + // These get rounded though + {"0.0001i", Quantity{dec(1, -3), DecimalSI}}, + {"0.005i", Quantity{dec(5, -3), DecimalSI}}, + {"0.05i", Quantity{dec(50, -3), DecimalSI}}, + // Also, if below you expect (512, -3), you're wrong in two ways: + // In the sequence [1024*1024*1024, 1024*1024, 1024, ?], the last term is "1" not 1/1024. + // Even if it were, 500 * 1/1024 = .48828125, NOT .512 + // I cannot recommend using this feature, it is confusing. + {"0.5i", Quantity{dec(500, -3), DecimalSI}}, } for _, item := range table { @@ -194,14 +211,6 @@ func TestQuantityParse(t *testing.T) { "1+1.0M", "0.1mi", "0.1am", - "0.0001i", - "100.035Ki", - "0.5Mi", - "0.05Gi", - "0.5Ti", - "0.005i", - "0.05i", - "0.5i", "aoeu", } for _, item := range invalid { @@ -223,17 +232,20 @@ func TestQuantityString(t *testing.T) { {Quantity{dec(1001*1024*1024*1024, 0), BinarySI}, "1001Gi"}, {Quantity{dec(1024*1024*1024*1024, 0), BinarySI}, "1Ti"}, {Quantity{dec(5, 0), BinarySI}, "5i"}, + {Quantity{dec(500, -3), BinarySI}, "500m"}, {Quantity{dec(1, 9), DecimalSI}, "1G"}, {Quantity{dec(1000, 6), DecimalSI}, "1G"}, {Quantity{dec(1000000, 3), DecimalSI}, "1G"}, {Quantity{dec(1000000000, 0), DecimalSI}, "1G"}, {Quantity{dec(1, -3), DecimalSI}, "1m"}, {Quantity{dec(80, -3), DecimalSI}, "80m"}, - {Quantity{dec(1080, -3), DecimalSI}, "1.080"}, - {Quantity{dec(108, -2), DecimalSI}, "1.080"}, - {Quantity{dec(10800, -4), DecimalSI}, "1.080"}, + {Quantity{dec(1080, -3), DecimalSI}, "1080m"}, + {Quantity{dec(108, -2), DecimalSI}, "1080m"}, + {Quantity{dec(10800, -4), DecimalSI}, "1080m"}, {Quantity{dec(300, 6), DecimalSI}, "300M"}, {Quantity{dec(1, 12), DecimalSI}, "1T"}, + {Quantity{dec(1234567, 6), DecimalSI}, "1234567M"}, + {Quantity{dec(1234567, -3), BinarySI}, "1235i"}, {Quantity{dec(3, 3), DecimalSI}, "3k"}, {Quantity{dec(0, 0), DecimalSI}, "0"}, {Quantity{dec(0, 0), BinarySI}, "0i"}, @@ -266,3 +278,150 @@ func TestQuantityString(t *testing.T) { } } } + +var fuzzer = fuzz.New().Funcs( + func(q *Quantity, c fuzz.Continue) { + q.Amount = &inf.Dec{} + if c.RandBool() { + q.Format = BinarySI + if c.RandBool() { + q.Amount.SetScale(0) + q.Amount.SetUnscaled(c.Int63()) + return + } + // Be sure to test cases like 1Mi + q.Amount.SetScale(0) + q.Amount.SetUnscaled(c.Int63n(1024) << uint(10*c.Intn(5))) + return + } + if c.RandBool() { + q.Format = DecimalSI + } else { + q.Format = DecimalExponent + } + if c.RandBool() { + q.Amount.SetScale(inf.Scale(c.Intn(4))) + q.Amount.SetUnscaled(c.Int63()) + return + } + // Be sure to test cases like 1M + q.Amount.SetScale(inf.Scale(3 - c.Intn(15))) + q.Amount.SetUnscaled(c.Int63n(1000)) + }, +) + +func TestJSON(t *testing.T) { + for i := 0; i < 500; i++ { + q := &Quantity{} + fuzzer.Fuzz(q) + b, err := json.Marshal(q) + if err != nil { + t.Errorf("error encoding %v", q) + } + q2 := &Quantity{} + err = json.Unmarshal(b, q2) + if err != nil { + t.Errorf("%v: error decoding %v", q, string(b)) + } + if q2.Amount.Cmp(q.Amount) != 0 { + t.Errorf("Expected equal: %v, %v (json was '%v')", q, q2, string(b)) + } + } +} + +func TestMilliNewSet(t *testing.T) { + table := []struct { + value int64 + format Format + expect string + exact bool + }{ + {1, DecimalSI, "1m", true}, + {1000, DecimalSI, "1", true}, + {1234000, DecimalSI, "1234", true}, + {1024, BinarySI, "2i", false}, // Rounded up + {1000000, "invalidFormatDefaultsToExponent", "1e3", true}, + {1024 * 1024, BinarySI, "1049i", false}, + } + + for _, item := range table { + q := NewMilliQuantity(item.value, item.format) + if e, a := item.expect, q.String(); e != a { + t.Errorf("Expected %v, got %v; %#v", e, a, q) + } + if !item.exact { + continue + } + q2, err := ParseQuantity(q.String()) + if err != nil { + t.Errorf("Round trip failed on %v", q) + } + if e, a := item.value, q2.MilliValue(); e != a { + t.Errorf("Expected %v, got %v", e, a) + } + } + + for _, item := range table { + q := NewQuantity(0, item.format) + q.SetMilli(item.value) + if e, a := item.expect, q.String(); e != a { + t.Errorf("Set: Expected %v, got %v; %#v", e, a, q) + } + } +} + +func TestNewSet(t *testing.T) { + table := []struct { + value int64 + format Format + expect string + }{ + {1, DecimalSI, "1"}, + {1000, DecimalSI, "1k"}, + {1234000, DecimalSI, "1234k"}, + {1024, BinarySI, "1Ki"}, + {1000000, "invalidFormatDefaultsToExponent", "1e6"}, + {1024 * 1024, BinarySI, "1Mi"}, + } + + for _, item := range table { + q := NewQuantity(item.value, item.format) + if e, a := item.expect, q.String(); e != a { + t.Errorf("Expected %v, got %v; %#v", e, a, q) + } + q2, err := ParseQuantity(q.String()) + if err != nil { + t.Errorf("Round trip failed on %v", q) + } + if e, a := item.value, q2.Value(); e != a { + t.Errorf("Expected %v, got %v", e, a) + } + } + + for _, item := range table { + q := NewQuantity(0, item.format) + q.Set(item.value) + if e, a := item.expect, q.String(); e != a { + t.Errorf("Set: Expected %v, got %v; %#v", e, a, q) + } + } +} + +func TestUninitializedNoCrash(t *testing.T) { + var q Quantity + + q.Value() + q.MilliValue() + q.Copy() + q.String() + q.MarshalJSON() +} + +func TestCopy(t *testing.T) { + q := NewQuantity(5, DecimalSI) + c := q.Copy() + c.Set(6) + if q.Value() == 6 { + t.Errorf("Copy didn't") + } +} diff --git a/pkg/api/resource/suffix.go b/pkg/api/resource/suffix.go index 818df999d50..a3a4012aa39 100644 --- a/pkg/api/resource/suffix.go +++ b/pkg/api/resource/suffix.go @@ -120,7 +120,7 @@ func (sh *suffixHandler) interpret(suffix suffix) (base, exponent int, fmt Forma return b, e, BinarySI, true } - if len(suffix) > 1 && suffix[0] == 'E' || suffix[0] == 'e' { + if len(suffix) > 1 && (suffix[0] == 'E' || suffix[0] == 'e') { parsed, err := strconv.ParseInt(string(suffix[1:]), 10, 64) if err != nil { return 0, 0, DecimalExponent, false