From f5eeb07bd2c6f32aaec2a6843a4a066a77acd3f4 Mon Sep 17 00:00:00 2001 From: Anders Eknert Date: Tue, 28 Oct 2025 22:30:52 +0100 Subject: [PATCH] Refactor hash key equality function (#7969) This was previously done in several places in a somewhat convoluted way, which probably made sense at some point. As expected however, a few small local variations had emerged, and while nothing critical, this code wasn't very nice to work with. Some comments stated as the rationale for the design was avoiding allocations, but those were nowhere to be seen when measured now, meaning there was no good reason to have it remain this way! It was *quite* nice to be able to merge the numbers comparsion functions (in particular) together into one! We now have a unified way for comparing Number values throughout the AST package, and as an added bonus, `1.0 == 1` is now true consistently for Rego. See for example @srenatus example in https://github.com/open-policy-agent/opa/issues/4797 ``` $ opa eval -fpretty 'count({1.0, 1})' 2 ``` Doing the same now gives: ``` $ go run main.go eval -fpretty 'count({1.0, 1})' 1 $ go run main.go eval -fpretty 'count({1.0, 1, 1.000, 1.00000})' 1 ``` What I have left out for now is however _presentation_. Meaning that even though 1.0 and 1 is now treated as the same value, you may still see either '1' or 1.0' (or whatever) displayed, depending on what was parsed. Should be easy to fix, but could perhaps be perceived as surprising... so holding off on that until we've had a discussion on the topic. Signed-off-by: Anders Eknert --- internal/edittree/edittree.go | 79 +------ v1/ast/compare.go | 193 ++++++++-------- v1/ast/compare_test.go | 7 + v1/ast/term.go | 404 ++++------------------------------ v1/ast/term_bench_test.go | 130 +++++++++++ 5 files changed, 286 insertions(+), 527 deletions(-) diff --git a/internal/edittree/edittree.go b/internal/edittree/edittree.go index 1dafc57b0b..ebfa875d75 100644 --- a/internal/edittree/edittree.go +++ b/internal/edittree/edittree.go @@ -148,7 +148,6 @@ package edittree import ( "errors" "fmt" - "math/big" "sort" "strings" @@ -203,89 +202,13 @@ func NewEditTree(term *ast.Term) *EditTree { // it was found in the table already. func (e *EditTree) getKeyHash(key *ast.Term) (int, bool) { hash := key.Hash() - // This `equal` utility is duplicated and manually inlined a number of - // time in this file. Inlining it avoids heap allocations, so it makes - // a big performance difference: some operations like lookup become twice - // as slow without it. - var equal func(v ast.Value) bool - - switch x := key.Value.(type) { - case ast.Null, ast.Boolean, ast.String, ast.Var: - equal = func(y ast.Value) bool { return x == y } - case ast.Number: - if xi, ok := x.Int64(); ok { - equal = func(y ast.Value) bool { - if y, ok := y.(ast.Number); ok { - if yi, ok := y.Int64(); ok { - return xi == yi - } - } - - return false - } - break - } - - // We use big.Rat for comparing big numbers. - // It replaces big.Float due to following reason: - // big.Float comes with a default precision of 64, and setting a - // larger precision results in more memory being allocated - // (regardless of the actual number we are parsing with SetString). - // - // Note: If we're so close to zero that big.Float says we are zero, do - // *not* big.Rat).SetString on the original string it'll potentially - // take very long. - var a *big.Rat - fa, ok := new(big.Float).SetString(string(x)) - if !ok { - panic("illegal value") - } - if fa.IsInt() { - if i, _ := fa.Int64(); i == 0 { - a = new(big.Rat).SetInt64(0) - } - } - if a == nil { - a, ok = new(big.Rat).SetString(string(x)) - if !ok { - panic("illegal value") - } - } - - equal = func(b ast.Value) bool { - if bNum, ok := b.(ast.Number); ok { - var b *big.Rat - fb, ok := new(big.Float).SetString(string(bNum)) - if !ok { - panic("illegal value") - } - if fb.IsInt() { - if i, _ := fb.Int64(); i == 0 { - b = new(big.Rat).SetInt64(0) - } - } - if b == nil { - b, ok = new(big.Rat).SetString(string(bNum)) - if !ok { - panic("illegal value") - } - } - - return a.Cmp(b) == 0 - } - return false - } - - default: - equal = func(y ast.Value) bool { return ast.Compare(x, y) == 0 } - } // Look through childKeys, looking up the original hash // value first, and then use linear-probing to iter // through the keys until we either find the Term we're // after, or run out of candidates. for curr, ok := e.childKeys[hash]; ok; { - if equal(curr.Value) { + if ast.KeyHashEqual(curr.Value, key.Value) { return hash, true } diff --git a/v1/ast/compare.go b/v1/ast/compare.go index c4754341de..663ad5ae05 100644 --- a/v1/ast/compare.go +++ b/v1/ast/compare.go @@ -5,9 +5,10 @@ package ast import ( - "encoding/json" + "cmp" "fmt" "math/big" + "strings" ) // Compare returns an integer indicating whether two AST values are less than, @@ -77,8 +78,7 @@ func Compare(a, b any) int { case Null: return 0 case Boolean: - b := b.(Boolean) - if a.Equal(b) { + if a == b.(Boolean) { return 0 } if !a { @@ -86,64 +86,10 @@ func Compare(a, b any) int { } return 1 case Number: - if ai, err := json.Number(a).Int64(); err == nil { - if bi, err := json.Number(b.(Number)).Int64(); err == nil { - if ai == bi { - return 0 - } - if ai < bi { - return -1 - } - return 1 - } - } - - // We use big.Rat for comparing big numbers. - // It replaces big.Float due to following reason: - // big.Float comes with a default precision of 64, and setting a - // larger precision results in more memory being allocated - // (regardless of the actual number we are parsing with SetString). - // - // Note: If we're so close to zero that big.Float says we are zero, do - // *not* big.Rat).SetString on the original string it'll potentially - // take very long. - var bigA, bigB *big.Rat - fa, ok := new(big.Float).SetString(string(a)) - if !ok { - panic("illegal value") - } - if fa.IsInt() { - if i, _ := fa.Int64(); i == 0 { - bigA = new(big.Rat).SetInt64(0) - } - } - if bigA == nil { - bigA, ok = new(big.Rat).SetString(string(a)) - if !ok { - panic("illegal value") - } - } - - fb, ok := new(big.Float).SetString(string(b.(Number))) - if !ok { - panic("illegal value") - } - if fb.IsInt() { - if i, _ := fb.Int64(); i == 0 { - bigB = new(big.Rat).SetInt64(0) - } - } - if bigB == nil { - bigB, ok = new(big.Rat).SetString(string(b.(Number))) - if !ok { - panic("illegal value") - } - } - - return bigA.Cmp(bigB) + return NumberCompare(a, b.(Number)) case String: b := b.(String) - if a.Equal(b) { + if a == b { return 0 } if a < b { @@ -153,8 +99,7 @@ func Compare(a, b any) int { case Var: return VarCompare(a, b.(Var)) case Ref: - b := b.(Ref) - return termSliceCompare(a, b) + return termSliceCompare(a, b.(Ref)) case *Array: b := b.(*Array) return termSliceCompare(a.elems, b.elems) @@ -164,11 +109,9 @@ func Compare(a, b any) int { if x, ok := b.(*lazyObj); ok { b = x.force() } - b := b.(*object) - return a.Compare(b) + return a.Compare(b.(*object)) case Set: - b := b.(Set) - return a.Compare(b) + return a.Compare(b.(Set)) case *ArrayComprehension: b := b.(*ArrayComprehension) if cmp := Compare(a.Term, b.Term); cmp != 0 { @@ -191,44 +134,31 @@ func Compare(a, b any) int { } return a.Body.Compare(b.Body) case Call: - b := b.(Call) - return termSliceCompare(a, b) + return termSliceCompare(a, b.(Call)) case *Expr: - b := b.(*Expr) - return a.Compare(b) + return a.Compare(b.(*Expr)) case *SomeDecl: - b := b.(*SomeDecl) - return a.Compare(b) + return a.Compare(b.(*SomeDecl)) case *Every: - b := b.(*Every) - return a.Compare(b) + return a.Compare(b.(*Every)) case *With: - b := b.(*With) - return a.Compare(b) + return a.Compare(b.(*With)) case Body: - b := b.(Body) - return a.Compare(b) + return a.Compare(b.(Body)) case *Head: - b := b.(*Head) - return a.Compare(b) + return a.Compare(b.(*Head)) case *Rule: - b := b.(*Rule) - return a.Compare(b) + return a.Compare(b.(*Rule)) case Args: - b := b.(Args) - return termSliceCompare(a, b) + return termSliceCompare(a, b.(Args)) case *Import: - b := b.(*Import) - return a.Compare(b) + return a.Compare(b.(*Import)) case *Package: - b := b.(*Package) - return a.Compare(b) + return a.Compare(b.(*Package)) case *Annotations: - b := b.(*Annotations) - return a.Compare(b) + return a.Compare(b.(*Annotations)) case *Module: - b := b.(*Module) - return a.Compare(b) + return a.Compare(b.(*Module)) } panic(fmt.Sprintf("illegal value: %T", a)) } @@ -427,3 +357,84 @@ func RefCompare(a, b Ref) int { func RefEqual(a, b Ref) bool { return termSliceEqual(a, b) } + +func NumberCompare(x, y Number) int { + xs, ys := string(x), string(y) + + var xIsF, yIsF bool + + // Treat "1" and "1.0", "1.00", etc as "1" + if strings.Contains(xs, ".") { + if tx := strings.TrimRight(xs, ".0"); tx != xs { + // Still a float after trimming? + xIsF = strings.Contains(tx, ".") + xs = tx + } + } + if strings.Contains(ys, ".") { + if ty := strings.TrimRight(ys, ".0"); ty != ys { + yIsF = strings.Contains(ty, ".") + ys = ty + } + } + if xs == ys { + return 0 + } + + var xi, yi int64 + var xf, yf float64 + var xiOK, yiOK, xfOK, yfOK bool + + if xi, xiOK = x.Int64(); xiOK { + if yi, yiOK = y.Int64(); yiOK { + return cmp.Compare(xi, yi) + } + } + + if xIsF && yIsF { + if xf, xfOK = x.Float64(); xfOK { + if yf, yfOK = y.Float64(); yfOK { + if xf == yf { + return 0 + } + // could still be "equal" depending on precision, so we continue? + } + } + } + + var a *big.Rat + fa, ok := new(big.Float).SetString(string(x)) + if !ok { + panic("illegal value") + } + if fa.IsInt() { + if i, _ := fa.Int64(); i == 0 { + a = new(big.Rat).SetInt64(0) + } + } + if a == nil { + a, ok = new(big.Rat).SetString(string(x)) + if !ok { + panic("illegal value") + } + } + + var b *big.Rat + fb, ok := new(big.Float).SetString(string(y)) + if !ok { + panic("illegal value") + } + if fb.IsInt() { + if i, _ := fb.Int64(); i == 0 { + b = new(big.Rat).SetInt64(0) + } + } + if b == nil { + b, ok = new(big.Rat).SetString(string(y)) + if !ok { + panic("illegal value") + } + } + + return a.Cmp(b) +} diff --git a/v1/ast/compare_test.go b/v1/ast/compare_test.go index ee5812be01..f8792328ed 100644 --- a/v1/ast/compare_test.go +++ b/v1/ast/compare_test.go @@ -30,8 +30,15 @@ func TestCompare(t *testing.T) { {"0", "1", -1}, {"1", "0", 1}, {"0", "0", 0}, + {"0.0", "0", 0}, + {"1.1", "1.10", 0}, + {"1", "1.0000000000000000000000000000000000000000000", 0}, + {"1.0", "1.0000000000000000000000000000000000000000000", 0}, + {"1.000000000000000", "1.0000000000000000000000000000000000000000000", 0}, + {"1.10", "1.11", -1}, {"0", "1.5", -1}, {"1.5", "0", 1}, + {"100000", "100", 1}, {"123456789123456789123", "123456789123456789123", 0}, {"123456789123456789123", "123456789123456789122", 1}, {"123456789123456789122", "123456789123456789123", -1}, diff --git a/v1/ast/term.go b/v1/ast/term.go index 046b01d9f9..18f8a423d9 100644 --- a/v1/ast/term.go +++ b/v1/ast/term.go @@ -12,7 +12,6 @@ import ( "fmt" "io" "math" - "math/big" "net/url" "regexp" "slices" @@ -669,22 +668,10 @@ func FloatNumberTerm(f float64) *Term { // Equal returns true if the other Value is a Number and is equal. func (num Number) Equal(other Value) bool { - switch other := other.(type) { - case Number: - if num == other { - return true - } - if n1, ok1 := num.Int64(); ok1 { - n2, ok2 := other.Int64() - if ok1 && ok2 { - return n1 == n2 - } - } - - return num.Compare(other) == 0 - default: - return false + if other, ok := other.(Number); ok { + return NumberCompare(num, other) == 0 } + return false } // Compare compares num to other, return <0, 0, or >0 if it is less than, equal to, @@ -692,17 +679,7 @@ func (num Number) Equal(other Value) bool { func (num Number) Compare(other Value) int { // Optimize for the common case, as calling Compare allocates on heap. if otherNum, yes := other.(Number); yes { - if ai, ok := num.Int64(); ok { - if bi, ok := otherNum.Int64(); ok { - if ai == bi { - return 0 - } - if ai < bi { - return -1 - } - return 1 - } - } + return NumberCompare(num, otherNum) } return Compare(num, other) @@ -723,13 +700,10 @@ func (num Number) Hash() int { return i } } - f, err := json.Number(num).Float64() - if err != nil { - bs := []byte(num) - h := xxhash.Sum64(bs) - return int(h) + if f, ok := num.Float64(); ok { + return int(f) } - return int(f) + return int(xxhash.Sum64String(string(num))) } // Int returns the int representation of num if possible. @@ -1792,85 +1766,9 @@ func (s *set) Slice() []*Term { func (s *set) insert(x *Term, resetSortGuard bool) { hash := x.Hash() insertHash := hash - // This `equal` utility is duplicated and manually inlined a number of - // time in this file. Inlining it avoids heap allocations, so it makes - // a big performance difference: some operations like lookup become twice - // as slow without it. - var equal func(v Value) bool - - switch x := x.Value.(type) { - case Null, Boolean, String, Var: - equal = func(y Value) bool { return x == y } - case Number: - if xi, err := json.Number(x).Int64(); err == nil { - equal = func(y Value) bool { - if y, ok := y.(Number); ok { - if yi, err := json.Number(y).Int64(); err == nil { - return xi == yi - } - } - - return false - } - break - } - - // We use big.Rat for comparing big numbers. - // It replaces big.Float due to following reason: - // big.Float comes with a default precision of 64, and setting a - // larger precision results in more memory being allocated - // (regardless of the actual number we are parsing with SetString). - // - // Note: If we're so close to zero that big.Float says we are zero, do - // *not* big.Rat).SetString on the original string it'll potentially - // take very long. - var a *big.Rat - fa, ok := new(big.Float).SetString(string(x)) - if !ok { - panic("illegal value") - } - if fa.IsInt() { - if i, _ := fa.Int64(); i == 0 { - a = new(big.Rat).SetInt64(0) - } - } - if a == nil { - a, ok = new(big.Rat).SetString(string(x)) - if !ok { - panic("illegal value") - } - } - - equal = func(b Value) bool { - if bNum, ok := b.(Number); ok { - var b *big.Rat - fb, ok := new(big.Float).SetString(string(bNum)) - if !ok { - panic("illegal value") - } - if fb.IsInt() { - if i, _ := fb.Int64(); i == 0 { - b = new(big.Rat).SetInt64(0) - } - } - if b == nil { - b, ok = new(big.Rat).SetString(string(bNum)) - if !ok { - panic("illegal value") - } - } - - return a.Cmp(b) == 0 - } - - return false - } - default: - equal = func(y Value) bool { return Compare(x, y) == 0 } - } for curr, ok := s.elems[insertHash]; ok; { - if equal(curr.Value) { + if KeyHashEqual(curr.Value, x.Value) { return } @@ -1896,87 +1794,18 @@ func (s *set) insert(x *Term, resetSortGuard bool) { } func (s *set) get(x *Term) *Term { - hash := x.Hash() - // This `equal` utility is duplicated and manually inlined a number of - // time in this file. Inlining it avoids heap allocations, so it makes - // a big performance difference: some operations like lookup become twice - // as slow without it. - var equal func(v Value) bool - - switch x := x.Value.(type) { - case Null, Boolean, String, Var: - equal = func(y Value) bool { return x == y } - case Number: - if xi, err := json.Number(x).Int64(); err == nil { - equal = func(y Value) bool { - if y, ok := y.(Number); ok { - if yi, err := json.Number(y).Int64(); err == nil { - return xi == yi - } - } - - return false - } - break - } - - // We use big.Rat for comparing big numbers. - // It replaces big.Float due to following reason: - // big.Float comes with a default precision of 64, and setting a - // larger precision results in more memory being allocated - // (regardless of the actual number we are parsing with SetString). - // - // Note: If we're so close to zero that big.Float says we are zero, do - // *not* big.Rat).SetString on the original string it'll potentially - // take very long. - var a *big.Rat - fa, ok := new(big.Float).SetString(string(x)) - if !ok { - panic("illegal value") - } - if fa.IsInt() { - if i, _ := fa.Int64(); i == 0 { - a = new(big.Rat).SetInt64(0) - } - } - if a == nil { - a, ok = new(big.Rat).SetString(string(x)) - if !ok { - panic("illegal value") - } - } - - equal = func(b Value) bool { - if bNum, ok := b.(Number); ok { - var b *big.Rat - fb, ok := new(big.Float).SetString(string(bNum)) - if !ok { - panic("illegal value") - } - if fb.IsInt() { - if i, _ := fb.Int64(); i == 0 { - b = new(big.Rat).SetInt64(0) - } - } - if b == nil { - b, ok = new(big.Rat).SetString(string(bNum)) - if !ok { - panic("illegal value") - } - } - - return a.Cmp(b) == 0 - } - return false - - } - - default: - equal = func(y Value) bool { return Compare(x, y) == 0 } + if len(s.elems) == 0 { + return nil } + hash := x.Hash() + for curr, ok := s.elems[hash]; ok; { - if equal(curr.Value) { + // Pointer equality check first + if curr == x { + return curr + } + if KeyHashEqual(curr.Value, x.Value) { return curr } @@ -2317,12 +2146,37 @@ func (obj *object) Insert(k, v *Term) { // Get returns the value of k in obj if k exists, otherwise nil. func (obj *object) Get(k *Term) *Term { - if elem := obj.get(k); elem != nil { - return elem.value + if len(obj.elems) == 0 { + return nil + } + + hash := k.Hash() + for curr := obj.elems[hash]; curr != nil; curr = curr.next { + // Pointer equality check always fastest, and not too unlikely with interning. + if curr.key == k { + return curr.value + } + + if KeyHashEqual(curr.key.Value, k.Value) { + return curr.value + } } return nil } +func KeyHashEqual(x, y Value) bool { + switch x := x.(type) { + case Null, Boolean, String, Var: + return x == y + case Number: + if y, ok := y.(Number); ok { + return x.Equal(y) + } + } + + return Compare(x, y) == 0 +} + // Hash returns the hash code for the Value. func (obj *object) Hash() int { return obj.hash @@ -2529,94 +2383,7 @@ func (obj *object) String() string { return sb.String() } -func (obj *object) get(k *Term) *objectElem { - hash := k.Hash() - - // This `equal` utility is duplicated and manually inlined a number of - // time in this file. Inlining it avoids heap allocations, so it makes - // a big performance difference: some operations like lookup become twice - // as slow without it. - var equal func(v Value) bool - - switch x := k.Value.(type) { - case Null, Boolean, String, Var: - equal = func(y Value) bool { return x == y } - case Number: - if xi, ok := x.Int64(); ok { - equal = func(y Value) bool { - if x == y { - return true - } - if y, ok := y.(Number); ok { - if yi, ok := y.Int64(); ok { - return xi == yi - } - } - - return false - } - break - } - - // We use big.Rat for comparing big numbers. - // It replaces big.Float due to following reason: - // big.Float comes with a default precision of 64, and setting a - // larger precision results in more memory being allocated - // (regardless of the actual number we are parsing with SetString). - // - // Note: If we're so close to zero that big.Float says we are zero, do - // *not* big.Rat).SetString on the original string it'll potentially - // take very long. - var a *big.Rat - fa, ok := new(big.Float).SetString(string(x)) - if !ok { - panic("illegal value") - } - if fa.IsInt() { - if i, _ := fa.Int64(); i == 0 { - a = new(big.Rat).SetInt64(0) - } - } - if a == nil { - a, ok = new(big.Rat).SetString(string(x)) - if !ok { - panic("illegal value") - } - } - - equal = func(b Value) bool { - if bNum, ok := b.(Number); ok { - var b *big.Rat - fb, ok := new(big.Float).SetString(string(bNum)) - if !ok { - panic("illegal value") - } - if fb.IsInt() { - if i, _ := fb.Int64(); i == 0 { - b = new(big.Rat).SetInt64(0) - } - } - if b == nil { - b, ok = new(big.Rat).SetString(string(bNum)) - if !ok { - panic("illegal value") - } - } - - return a.Cmp(b) == 0 - } - - return false - } - default: - equal = func(y Value) bool { return Compare(x, y) == 0 } - } - - for curr := obj.elems[hash]; curr != nil; curr = curr.next { - if equal(curr.key.Value) { - return curr - } - } +func (*object) get(*Term) *objectElem { return nil } @@ -2625,88 +2392,9 @@ func (obj *object) get(k *Term) *objectElem { func (obj *object) insert(k, v *Term, resetSortGuard bool) { hash := k.Hash() head := obj.elems[hash] - // This `equal` utility is duplicated and manually inlined a number of - // time in this file. Inlining it avoids heap allocations, so it makes - // a big performance difference: some operations like lookup become twice - // as slow without it. - var equal func(v Value) bool - - switch x := k.Value.(type) { - case Null, Boolean, String, Var: - equal = func(y Value) bool { return x == y } - case Number: - if xi, err := json.Number(x).Int64(); err == nil { - equal = func(y Value) bool { - if x == y { - return true - } - if y, ok := y.(Number); ok { - if yi, err := json.Number(y).Int64(); err == nil { - return xi == yi - } - } - - return false - } - break - } - - // We use big.Rat for comparing big numbers. - // It replaces big.Float due to following reason: - // big.Float comes with a default precision of 64, and setting a - // larger precision results in more memory being allocated - // (regardless of the actual number we are parsing with SetString). - // - // Note: If we're so close to zero that big.Float says we are zero, do - // *not* big.Rat).SetString on the original string it'll potentially - // take very long. - var a *big.Rat - fa, ok := new(big.Float).SetString(string(x)) - if !ok { - panic("illegal value") - } - if fa.IsInt() { - if i, _ := fa.Int64(); i == 0 { - a = new(big.Rat).SetInt64(0) - } - } - if a == nil { - a, ok = new(big.Rat).SetString(string(x)) - if !ok { - panic("illegal value") - } - } - - equal = func(b Value) bool { - if bNum, ok := b.(Number); ok { - var b *big.Rat - fb, ok := new(big.Float).SetString(string(bNum)) - if !ok { - panic("illegal value") - } - if fb.IsInt() { - if i, _ := fb.Int64(); i == 0 { - b = new(big.Rat).SetInt64(0) - } - } - if b == nil { - b, ok = new(big.Rat).SetString(string(bNum)) - if !ok { - panic("illegal value") - } - } - - return a.Cmp(b) == 0 - } - - return false - } - default: - equal = func(y Value) bool { return Compare(x, y) == 0 } - } for curr := head; curr != nil; curr = curr.next { - if equal(curr.key.Value) { + if KeyHashEqual(curr.key.Value, k.Value) { if curr.value.IsGround() { obj.ground-- } diff --git a/v1/ast/term_bench_test.go b/v1/ast/term_bench_test.go index 2934d98048..70f90b903a 100644 --- a/v1/ast/term_bench_test.go +++ b/v1/ast/term_bench_test.go @@ -41,6 +41,136 @@ func BenchmarkObjectLookup(b *testing.B) { } } +// Before NumberCompare refactor: +// // --- FAIL: BenchmarkObjectGet/existing_float_number_key_as_int +// /Users/anderseknert/git/opa/opa/v1/ast/term_bench_test.go:111: expected hit + +// BenchmarkObjectGet/lookup_in_empty_object-16 219916140 5.323 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/existing_interned_key-16 149059920 8.011 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/existing_string_key-16 144672567 8.314 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/existing_int_number_key-16 62073110 17.62 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/existing_int_number_key_as_float-16 2310716 519.5 ns/op 504 B/op 24 allocs/op +// BenchmarkObjectGet/existing_float_key-16 1966604 611.5 ns/op 632 B/op 29 allocs/op +// BenchmarkObjectGet/missing_string_key-16 164003106 7.293 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/missing_int_number_key-16 74759754 15.25 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/missing_float_key-16 4059250 295.8 ns/op 296 B/op 15 allocs/op + +// After NumberCompare refactor: +// BenchmarkObjectGet/lookup_in_empty_object-16 680466268 1.767 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/existing_interned_key-16 263787909 4.498 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/existing_string_key-16 156048259 7.646 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/existing_int_number_key-16 99076318 12.40 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/existing_float_number_key_as_int-16 98104674 12.47 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/existing_int_number_key_as_float-16 53441701 22.83 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/existing_float_key-16 53429703 20.98 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/missing_string_key-16 193661902 6.084 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/missing_int_number_key-16 156364982 7.695 ns/op 0 B/op 0 allocs/op +// BenchmarkObjectGet/missing_float_key-16 67888981 16.26 ns/op 0 B/op 0 allocs/op + +// But do note that this was not done to improve performance, but to improve our code. The faster float comparisons +// are a nice side effect. + +func BenchmarkObjectGet(b *testing.B) { + obj := NewObject( + Item(InternedTerm("env"), InternedTerm("production")), // known interned string key + Item(StringTerm("a"), InternedTerm(1)), + Item(IntNumberTerm(222), InternedTerm("b")), + Item(NumberTerm("3.14"), InternedTerm("c")), + Item(NumberTerm("2.0"), InternedTerm("d")), + ) + + empty := NewObject() + b.Run("lookup in empty object", func(b *testing.B) { + key := StringTerm("a") + for b.Loop() { + if empty.Get(key) != nil { + b.Fatal("expected miss") + } + } + }) + + b.Run("existing interned key", func(b *testing.B) { + key := InternedTerm("env") + for b.Loop() { + if obj.Get(key) == nil { + b.Fatal("expected hit") + } + } + }) + + b.Run("existing string key", func(b *testing.B) { + key := StringTerm("a") + for b.Loop() { + if obj.Get(key) == nil { + b.Fatal("expected hit") + } + } + }) + + b.Run("existing int number key", func(b *testing.B) { + key := IntNumberTerm(222) + for b.Loop() { + if obj.Get(key) == nil { + b.Fatal("expected hit") + } + } + }) + + b.Run("existing float number key as int", func(b *testing.B) { + key := IntNumberTerm(2) + for b.Loop() { + if obj.Get(key) == nil { + b.Fatal("expected hit") + } + } + }) + + b.Run("existing int number key as float", func(b *testing.B) { + key := NumberTerm("222.0") + for b.Loop() { + if obj.Get(key) == nil { + b.Fatal("expected hit") + } + } + }) + + b.Run("existing float key", func(b *testing.B) { + key := NumberTerm("3.14") + for b.Loop() { + if obj.Get(key) == nil { + b.Fatal("expected hit") + } + } + }) + + b.Run("missing string key", func(b *testing.B) { + key := StringTerm("missing") + for b.Loop() { + if obj.Get(key) != nil { + b.Fatal("expected miss") + } + } + }) + + b.Run("missing int number key", func(b *testing.B) { + key := IntNumberTerm(999) + for b.Loop() { + if obj.Get(key) != nil { + b.Fatal("expected miss") + } + } + }) + + b.Run("missing float key", func(b *testing.B) { + key := NumberTerm("9.99") + for b.Loop() { + if obj.Get(key) != nil { + b.Fatal("expected miss") + } + } + }) +} + func BenchmarkObjectFind(b *testing.B) { sizes := []int{5, 50, 500, 5000} for _, n := range sizes {