Skip to content

Commit

Permalink
update to cockroachdb/apd v3
Browse files Browse the repository at this point in the history
The main breaking change that apd/v3 brings is that it no longer uses
math/big types directly. There are new wrapper types which avoid
allocations in many common cases. See the added calls to MathBigInt and
SetMathBigInt.

The other breaking change happened in Quo and Sqrt,
causing the operations in CUE to end up with different precision:

	=== RUN   TestMarshalJSON
	=== RUN   TestMarshalJSON/22/a:_1.0/1
		types_test.go:2758:
			 got {"a":1.00000000000000000000000};
			want {"a":1.0}
	--- FAIL: TestMarshalJSON (0.00s)
		--- FAIL: TestMarshalJSON/22/a:_1.0/1 (0.00s)

Get the old behavior back by overwriting the Quo and Sqrt methods,
running a short piece of code to do what v2 used to do.
This makes the unit tests pass again.

As a small bonus, whenever we use Quo or Rem on an apd float,
the resulting Decimal is now consistently reduced,
leaving at least one digit after the decimal point.

For example, cue/testdata/resolve/006_arithmetic.txtar worked out
"4.0 / 2.0" as "2", but now it is "2.0".
Similarly, cue/testdata/basicrewrite/002_arithmetic.txtar worked out
"1.0T / 2.0" as "5.0000000000E+11", but now it is "5.0E+11".

To properly change the behavior of Quo and Rem across the codebase,
internal.BaseContext is now of type internal.Context,
which embeds apd.Context. internal.BaseContext is now used everywhere,
except for cue/literal, as that would result in cyclic imports.
For now that is fine, since that package uses neither Quo nor Rem.

If we want cue/literal to use the same custom apd.Context as the rest of
the codebase, we could always move internal.Context to avoid the cycle.

Performance is largely unchanged; there are likely more gains to be made
if we reduce the amount of times we convert to math/big types.

    goos: linux
    goarch: amd64
    pkg: cuelang.org/go/cue/testdata/benchmarks
    cpu: AMD Ryzen 7 PRO 5850U with Radeon Graphics
                     │  apd-v2-24  │             apd-v3-24             │
                     │   sec/op    │   sec/op     vs base              │
    /decimal.txtar-8   4.982m ± 1%   5.045m ± 0%  +1.27% (p=0.004 n=6)

                     │  apd-v2-24   │             apd-v3-24              │
                     │     B/op     │     B/op      vs base              │
    /decimal.txtar-8   1.873Mi ± 0%   1.872Mi ± 0%  -0.09% (p=0.002 n=6)

                     │  apd-v2-24  │             apd-v3-24             │
                     │  allocs/op  │  allocs/op   vs base              │
    /decimal.txtar-8   29.72k ± 0%   29.65k ± 0%  -0.22% (p=0.002 n=6)

Fixes #1872.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: Iadcb4a50f7c9943601a1124570387f62ed65a6f5
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/543421
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Marcel van Lohuizen <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
  • Loading branch information
mvdan committed Jun 15, 2023
1 parent f3be10a commit 4728223
Show file tree
Hide file tree
Showing 23 changed files with 135 additions and 96 deletions.
3 changes: 2 additions & 1 deletion cue/literal/num.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ package literal
import (
"cuelang.org/go/cue/errors"
"cuelang.org/go/cue/token"
"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/apd/v3"
)

// We avoid cuelang.org/go/internal.Context as that would be an import cycle.
var baseContext apd.Context

func init() {
Expand Down
2 changes: 1 addition & 1 deletion cue/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"cuelang.org/go/cue/token"
"cuelang.org/go/internal/astinternal"
"cuelang.org/go/internal/core/adt"
"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/apd/v3"
)

// SelectorType represents the kind of a selector. It indicates both the label
Expand Down
4 changes: 2 additions & 2 deletions cue/testdata/basicrewrite/002_arithmetic.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ Result:
sum: (int){ 1 }
div1: (float){ 4.00000000000000000000000 }
div2: (float){ 4.00000000000000000000000 }
div3: (float){ 1 }
div3: (float){ 1.0 }
divZero: (_|_){
// [eval] divZero: failed arithmetic: division by zero:
// ./in.cue:8:10
Expand All @@ -168,7 +168,7 @@ Result:
// [eval] irem00: division by zero:
// ./in.cue:16:9
}
v1: (float){ 5.0000000000E+11 }
v1: (float){ 5.0E+11 }
v2: (bool){ true }
v3: (float){ 0.666666666666666666666667 }
v5: (int){ 0 }
Expand Down
2 changes: 1 addition & 1 deletion cue/testdata/benchmarks/decimal.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ Disjuncts: 11
4: (int){ 5 }
5: (int){ 6 }
6: (int){ 50 }
7: (float){ 5 }
7: (float){ 5.0 }
8: (float){ 0.333333333333333333333333 }
}
}
8 changes: 4 additions & 4 deletions cue/testdata/resolve/006_arithmetic.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,20 @@ Conjuncts: 8
Disjuncts: 7
-- out/eval --
Errors:
e2: conflicting values int and 2 (mismatched types int and float):
e2: conflicting values int and 2.0 (mismatched types int and float):
./in.cue:6:5
./in.cue:6:11

Result:
(_|_){
// [eval]
v1: (float){ 5.0000000000E+11 }
v1: (float){ 5.0E+11 }
v2: (bool){ true }
n1: (int){ 1 }
v5: (float){ 2.0 }
v6: (float){ 1 }
v6: (float){ 1.0 }
e2: (_|_){
// [eval] e2: conflicting values int and 2 (mismatched types int and float):
// [eval] e2: conflicting values int and 2.0 (mismatched types int and float):
// ./in.cue:6:5
// ./in.cue:6:11
}
Expand Down
15 changes: 7 additions & 8 deletions cue/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"math/big"
"strings"

"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/apd/v3"

"cuelang.org/go/cue/ast"
"cuelang.org/go/cue/build"
Expand Down Expand Up @@ -335,7 +335,7 @@ func (v Value) MantExp(mant *big.Int) (exp int, err error) {
return 0, ErrInfinite
}
if mant != nil {
mant.Set(&n.X.Coeff)
mant.Set(n.X.Coeff.MathBigInt())
if n.X.Negative {
mant.Neg(mant)
}
Expand Down Expand Up @@ -371,7 +371,7 @@ func (v Value) AppendFloat(buf []byte, fmt byte, prec int) ([]byte, error) {
if err != nil {
return nil, err
}
ctx := apd.BaseContext
ctx := internal.BaseContext
nd := int(apd.NumDigits(&n.X.Coeff)) + int(n.X.Exponent)
if n.X.Form == apd.Infinite {
if n.X.Negative {
Expand All @@ -380,9 +380,9 @@ func (v Value) AppendFloat(buf []byte, fmt byte, prec int) ([]byte, error) {
return append(buf, string('∞')...), nil
}
if fmt == 'f' && nd > 0 {
ctx.Precision = uint32(nd + prec)
ctx = ctx.WithPrecision(uint32(nd + prec))
} else {
ctx.Precision = uint32(prec)
ctx = ctx.WithPrecision(uint32(prec))
}
var d apd.Decimal
ctx.Round(&d, &n.X)
Expand Down Expand Up @@ -415,7 +415,7 @@ func (v Value) Int(z *big.Int) (*big.Int, error) {
if n.X.Exponent != 0 {
panic("cue: exponent should always be nil for integer types")
}
z.Set(&n.X.Coeff)
z.Set(n.X.Coeff.MathBigInt())
if n.X.Negative {
z.Neg(z)
}
Expand Down Expand Up @@ -494,8 +494,7 @@ func init() {
// math.MaxFloat64: 2**1023 * (2**53 - 1) / 2**52
max = "1.797693134862315708145274237317043567981e+308"
)
ctx := apd.BaseContext
ctx.Precision = 40
ctx := internal.BaseContext.WithPrecision(40)

var err error
smallestPosFloat64, _, err = ctx.NewFromString(smallest)
Expand Down
2 changes: 1 addition & 1 deletion encoding/openapi/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ package openapi
import (
"fmt"

"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/apd/v3"

"cuelang.org/go/cue"
"cuelang.org/go/cue/ast"
Expand Down
2 changes: 1 addition & 1 deletion encoding/protobuf/jsonpb/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"cuelang.org/go/cue/literal"
"cuelang.org/go/cue/token"
"cuelang.org/go/encoding/protobuf/pbinternal"
"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/apd/v3"
)

// Option is an option.
Expand Down
4 changes: 1 addition & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module cuelang.org/go
go 1.19

require (
github.com/cockroachdb/apd/v2 v2.0.2
github.com/cockroachdb/apd/v3 v3.2.0
github.com/emicklei/proto v1.10.0
github.com/go-quicktest/qt v1.100.0
github.com/google/go-cmp v0.5.9
Expand All @@ -27,9 +27,7 @@ require (
require (
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/lib/pq v1.0.0 // indirect
github.com/mitchellh/go-wordwrap v1.0.1 // indirect
github.com/pkg/errors v0.8.1 // indirect
golang.org/x/sys v0.6.0 // indirect
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect
)
10 changes: 3 additions & 7 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
github.com/cockroachdb/apd/v2 v2.0.2 h1:weh8u7Cneje73dDh+2tEVLUvyBc89iwepWCD8b8034E=
github.com/cockroachdb/apd/v2 v2.0.2/go.mod h1:DDxRlzC2lo3/vSlmSoS7JkqbbrARPuFOGr0B9pvN3Gw=
github.com/cockroachdb/apd/v3 v3.2.0 h1:79kHCn4tO0VGu3W0WujYrMjBDk8a2H4KEUYcXf7whcg=
github.com/cockroachdb/apd/v3 v3.2.0/go.mod h1:klXJcjp+FffLTHlhIG69tezTDvdP065naDsHzKhYSqc=
github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/emicklei/proto v1.10.0 h1:pDGyFRVV5RvV+nkBK9iy3q67FBy9Xa7vwrOTE+g5aGw=
Expand All @@ -20,16 +20,12 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
github.com/lib/pq v1.0.0 h1:X5PMW56eZitiTeO7tKzZxFCSpbFZJtkMMooicw2us9A=
github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
github.com/lib/pq v1.10.7 h1:p7ZhMD+KsSRozJr34udlUrhboJwWAgCg34+/ZZNvZZw=
github.com/mitchellh/go-wordwrap v1.0.1 h1:TLuKupo69TCn6TQSyGxwI1EblZZEsQ0vMlAFQflz0v0=
github.com/mitchellh/go-wordwrap v1.0.1/go.mod h1:R62XHJLzvMFRBbcrT7m7WgmE1eOyTSsCt+hzestvNj0=
github.com/mpvl/unique v0.0.0-20150818121801-cbe035fff7de h1:D5x39vF5KCwKQaw+OC9ZPiLVHXz3UFw2+psEX+gYcto=
github.com/mpvl/unique v0.0.0-20150818121801-cbe035fff7de/go.mod h1:kJun4WP5gFuHZgRjZUWWuH1DTxCtxbHDOIJsudS8jzY=
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/protocolbuffers/txtpbfmt v0.0.0-20230328191034-3462fbc510c0 h1:sadMIsgmHpEOGbUs6VtHBXRR1OHevnj7hLx9ZcdNGW4=
github.com/protocolbuffers/txtpbfmt v0.0.0-20230328191034-3462fbc510c0/go.mod h1:jgxiZysxFPM+iWKwQwPR+y+Jvo54ARd4EisXxKYpB5c=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"sort"
"strings"

"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/apd/v3"
"golang.org/x/text/encoding/unicode"

"cuelang.org/go/cue/ast"
Expand Down
36 changes: 14 additions & 22 deletions internal/core/adt/decimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,10 @@
package adt

import (
"math/big"

"github.com/cockroachdb/apd/v2"
"cuelang.org/go/internal"
"github.com/cockroachdb/apd/v3"
)

var apdCtx apd.Context

func init() {
apdCtx = apd.BaseContext
apdCtx.Precision = 24
}

func (n *Num) Impl() *apd.Decimal {
return &n.X
}
Expand All @@ -40,27 +32,27 @@ func (a *Num) Cmp(b *Num) int {
}

func (c *OpContext) Add(a, b *Num) Value {
return numOp(c, apdCtx.Add, a, b)
return numOp(c, internal.BaseContext.Add, a, b)
}

func (c *OpContext) Sub(a, b *Num) Value {
return numOp(c, apdCtx.Sub, a, b)
return numOp(c, internal.BaseContext.Sub, a, b)
}

func (c *OpContext) Mul(a, b *Num) Value {
return numOp(c, apdCtx.Mul, a, b)
return numOp(c, internal.BaseContext.Mul, a, b)
}

func (c *OpContext) Quo(a, b *Num) Value {
v := numOp(c, apdCtx.Quo, a, b)
v := numOp(c, internal.BaseContext.Quo, a, b)
if n, ok := v.(*Num); ok {
n.K = FloatKind
}
return v
}

func (c *OpContext) Pow(a, b *Num) Value {
return numOp(c, apdCtx.Pow, a, b)
return numOp(c, internal.BaseContext.Pow, a, b)
}

type numFunc func(z, x, y *apd.Decimal) (apd.Condition, error)
Expand All @@ -86,34 +78,34 @@ func numOp(c *OpContext, fn numFunc, x, y *Num) Value {
}

func (c *OpContext) IntDiv(a, b *Num) Value {
return intDivOp(c, (*big.Int).Div, a, b)
return intDivOp(c, (*apd.BigInt).Div, a, b)
}

func (c *OpContext) IntMod(a, b *Num) Value {
return intDivOp(c, (*big.Int).Mod, a, b)
return intDivOp(c, (*apd.BigInt).Mod, a, b)
}

func (c *OpContext) IntQuo(a, b *Num) Value {
return intDivOp(c, (*big.Int).Quo, a, b)
return intDivOp(c, (*apd.BigInt).Quo, a, b)
}

func (c *OpContext) IntRem(a, b *Num) Value {
return intDivOp(c, (*big.Int).Rem, a, b)
return intDivOp(c, (*apd.BigInt).Rem, a, b)
}

type intFunc func(z, x, y *big.Int) *big.Int
type intFunc func(z, x, y *apd.BigInt) *apd.BigInt

func intDivOp(c *OpContext, fn intFunc, a, b *Num) Value {
if b.X.IsZero() {
return c.NewErrf("division by zero")
}

var x, y apd.Decimal
_, _ = apdCtx.RoundToIntegralValue(&x, &a.X)
_, _ = internal.BaseContext.RoundToIntegralValue(&x, &a.X)
if x.Negative {
x.Coeff.Neg(&x.Coeff)
}
_, _ = apdCtx.RoundToIntegralValue(&y, &b.X)
_, _ = internal.BaseContext.RoundToIntegralValue(&y, &b.X)
if y.Negative {
y.Coeff.Neg(&y.Coeff)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"io"
"regexp"

"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/apd/v3"

"cuelang.org/go/cue/ast"
"cuelang.org/go/cue/errors"
Expand Down
16 changes: 9 additions & 7 deletions internal/core/adt/simplify.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
package adt

import (
"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/apd/v3"

"cuelang.org/go/internal"
)

// SimplifyBounds collapses bounds if possible. The bound values must be
Expand Down Expand Up @@ -82,21 +84,21 @@ func SimplifyBounds(ctx *OpContext, k Kind, x, y *BoundValue) Value {
// Readjust bounds for integers.
if x.Op == GreaterEqualOp {
// >=3.4 ==> >=4
_, _ = apdCtx.Ceil(&lo, &a.X)
_, _ = internal.BaseContext.Ceil(&lo, &a.X)
} else {
// >3.4 ==> >3
_, _ = apdCtx.Floor(&lo, &a.X)
_, _ = internal.BaseContext.Floor(&lo, &a.X)
}
if y.Op == LessEqualOp {
// <=2.3 ==> <= 2
_, _ = apdCtx.Floor(&hi, &b.X)
_, _ = internal.BaseContext.Floor(&hi, &b.X)
} else {
// <2.3 ==> < 3
_, _ = apdCtx.Ceil(&hi, &b.X)
_, _ = internal.BaseContext.Ceil(&hi, &b.X)
}
}

cond, err := apd.BaseContext.Sub(&d, &hi, &lo)
cond, err := internal.BaseContext.Sub(&d, &hi, &lo)
if cond.Inexact() || err != nil {
break
}
Expand Down Expand Up @@ -140,7 +142,7 @@ func SimplifyBounds(ctx *OpContext, k Kind, x, y *BoundValue) Value {

case diff == 2:
if k&FloatKind == 0 && x.Op == GreaterThanOp && y.Op == LessThanOp {
_, _ = apd.BaseContext.Add(&d, d.SetInt64(1), &lo)
_, _ = internal.BaseContext.Add(&d, d.SetInt64(1), &lo)
return ctx.newNum(&d, k&NumKind, x, y)

}
Expand Down
2 changes: 1 addition & 1 deletion internal/core/compile/label.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package compile

import (
"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/apd/v3"
"golang.org/x/text/unicode/norm"

"cuelang.org/go/cue/ast"
Expand Down
Loading

0 comments on commit 4728223

Please sign in to comment.