ewilliams28/releases
1
0
Fork 0
mirror of https://github.com/open-policy-agent/opa.git synced 2026-08-12 19:32:48 -06:00
Code Issues Packages Projects Releases Wiki Activity

perf: avoid allocations with custom Atoi and Atoi64 helpers (#8758)

Browse Source 
strconv.Atoi was called frequently in OPA, and its failure case is
expensive. This custom implementation is slightly faster for the
successful case, but more importantly much more efficient in the failure
case, allocating nothing for any given input string.

Signed-off-by: Anders Eknert <anders.eknert@apple.com>
This commit is contained in:
Anders Eknert
2026-07-02 12:14:17 +02:00
committed by GitHub
parent e5679d92ce
commit b4a7a19882
10 changed files with 252 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
v1/ast/parser.go
+1 -2
View File
@@ -3091,8 +3091,7 @@ func (b *metadataParser) Parse() (result *Annotations, err error) {
var comment *Comment
match := yamlLineErrRegex.FindStringSubmatch(err.Error())
if len(match) == 2 {
index, err2 := strconv.Atoi(match[1])
if err2 == nil {
if index, ok := util.Atoi(match[1]); ok {
if index >= len(b.comments) {
comment = b.comments[len(b.comments)-1]
} else {
v1/ast/term.go
+2 -6
View File
@@ -871,7 +871,7 @@ func (num Number) Find(path Ref) (Value, error) {
// Hash returns the hash code for the Value.
func (num Number) Hash() int {
if len(num) < 4 {
if i, err := strconv.Atoi(string(num)); err == nil {
if i, ok := util.Atoi(string(num)); ok {
return i
}
}
@@ -889,11 +889,7 @@ func (num Number) Int() (int, bool) {
// Int64 returns the int64 representation of num if possible.
func (num Number) Int64() (int64, bool) {
i, err := json.Number(num).Int64()
if err != nil {
return 0, false
}
return i, true
return util.Atoi64(string(num))
}
// Float64 returns the float64 representation of num if possible.
v1/ast/term_bench_test.go
+22
View File
@@ -1133,3 +1133,25 @@ func BenchmarkTemplateStringToStringEscapeControl(b *testing.B) {
b.Fatalf("expected %q but got %q", exp, s)
}
}
// 13.00 ns/op 0 B/op 0 allocs/op // json.Number.Int64()
// 4.738 ns/op 0 B/op 0 allocs/op // util.Atoi64()
func BenchmarkNumberInt64(b *testing.B) {
num := Number("1234567890")
for b.Loop() {
if x, ok := num.Int64(); !ok || x != 1234567890 {
b.Fatalf("expected %d but got %d", 1234567890, x)
}
}
}
// 35.42 ns/op 64 B/op 2 allocs/op // json.Number.Int64()
// 2.549 ns/op 0 B/op 0 allocs/op // util.Atoi64()
func BenchmarkNumberInt64Fail(b *testing.B) {
num := Number("12345.67890")
for b.Loop() {
if _, ok := num.Int64(); ok {
b.Fatal("expected failure")
}
}
}
v1/server/server.go
+4 -3
View File
@@ -2847,9 +2847,10 @@ func stringPathToRef(s string) (ast.Ref, error) {
return nil, fmt.Errorf("invalid ref term '%s'", x)
}
i, err := strconv.Atoi(x)
if err != nil {
r = append(r, ast.StringTerm(x))
// Note(anders): the branches look identical, but the difference
// in type decides where we go to look for an interned term
if i, ok := util.Atoi64(x); !ok {
r = append(r, ast.InternedTerm(x))
} else {
r = append(r, ast.InternedTerm(i))
}
v1/storage/internal/ptr/ptr.go
+4 -10
View File
@@ -6,11 +6,10 @@
package ptr
import (
"strconv"
"github.com/open-policy-agent/opa/v1/ast"
"github.com/open-policy-agent/opa/v1/storage"
"github.com/open-policy-agent/opa/v1/storage/internal/errors"
"github.com/open-policy-agent/opa/v1/util"
)
func Ptr(data any, path storage.Path) (any, error) {
@@ -92,7 +91,7 @@ func ValuePtr(data ast.Value, path storage.Path) (ast.Value, error) {
}
func ValidateArrayIndex(arr []any, s string, path storage.Path) (int, error) {
idx, ok := isInt(s)
idx, ok := util.Atoi(s)
if !ok {
return 0, errors.NewNotFoundErrorWithHint(path, errors.ArrayIndexTypeMsg)
}
@@ -100,7 +99,7 @@ func ValidateArrayIndex(arr []any, s string, path storage.Path) (int, error) {
}
func ValidateASTArrayIndex(arr *ast.Array, s string, path storage.Path) (int, error) {
idx, ok := isInt(s)
idx, ok := util.Atoi(s)
if !ok {
return 0, errors.NewNotFoundErrorWithHint(path, errors.ArrayIndexTypeMsg)
}
@@ -111,18 +110,13 @@ func ValidateASTArrayIndex(arr *ast.Array, s string, path storage.Path) (int, er
// array element like `ValidateArrayIndex`, but returns a `resource_conflict` error
// if it is not.
func ValidateArrayIndexForWrite(arr []any, s string, i int, path storage.Path) (int, error) {
idx, ok := isInt(s)
idx, ok := util.Atoi(s)
if !ok {
return 0, errors.NewWriteConflictError(path[:i-1])
}
return inRange(idx, arr, path)
}
func isInt(s string) (int, bool) {
idx, err := strconv.Atoi(s)
return idx, err == nil
}
func inRange(i int, arr any, path storage.Path) (int, error) {
var arrLen int
v1/topdown/strings.go
+26 -15
View File
@@ -722,15 +722,15 @@ func builtinSprintf(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term)
return err
}
astArr, ok := operands[1].Value.(*ast.Array)
if !ok {
return builtins.NewOperandTypeErr(2, operands[1].Value, "array")
a, err := builtins.ArrayOperand(operands[1].Value, 2)
if err != nil {
return err
}
// Optimized path for where sprintf is used as a "to_string" function for
// a single integer, i.e. sprintf("%d", [x]) where x is an integer.
if s == "%d" && astArr.Len() == 1 {
if n, ok := astArr.Elem(0).Value.(ast.Number); ok {
if s == "%d" && a.Len() == 1 {
if n, ok := a.Elem(0).Value.(ast.Number); ok {
if i, ok := n.Int(); ok {
if interned := ast.InternedIntegerString(i); interned != nil {
return iter(interned)
@@ -740,24 +740,35 @@ func builtinSprintf(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term)
}
}
args := make([]any, astArr.Len())
args := make([]any, a.Len())
for i := range args {
switch v := astArr.Elem(i).Value.(type) {
t := a.Elem(i)
switch v := t.Value.(type) {
case ast.Number:
if n, ok := v.Int(); ok {
args[i] = n
} else if b, ok := new(big.Int).SetString(v.String(), 10); ok {
args[i] = b
} else if f, ok := v.Float64(); ok {
args[i] = f
ns := string(v)
if x, ok := util.Atoi64(ns); ok {
args[i] = x
} else {
args[i] = v.String()
if strings.ContainsRune(ns, '.') {
if f, ok := v.Float64(); ok {
args[i] = f
continue
} else {
args[i] = ns
}
} else {
if b, ok := new(big.Int).SetString(ns, 10); ok {
args[i] = b
} else {
args[i] = ns
}
}
}
case ast.String:
args[i] = string(v)
default:
args[i] = astArr.Elem(i).String()
args[i] = t.Value.String()
}
}
v1/topdown/strings_bench_test.go
+29 -2
View File
@@ -331,8 +331,6 @@ func BenchmarkConcatVsSprintfSimple(b *testing.B) {
}
})
b.ResetTimer()
b.Run("sprintf foobar", func(b *testing.B) {
operands := []*ast.Term{ast.InternedTerm("%s%s"), ast.ArrayTerm(foo, bar)}
@@ -344,6 +342,35 @@ func BenchmarkConcatVsSprintfSimple(b *testing.B) {
})
}
// 542.1 ns/op 552 B/op 20 allocs/op
// 451.5 ns/op 424 B/op 15 allocs/op
func BenchmarkLongSprintf(b *testing.B) {
bctx := BuiltinContext{}
operands := []*ast.Term{
ast.StringTerm("%s %f %s %d %s %v %s %v %s"),
ast.ArrayTerm(
ast.InternedTerm("word"),
ast.FloatNumberTerm(3.14159),
ast.InternedTerm("more"),
ast.InternedTerm(55),
ast.ArrayTerm(ast.InternedTerm("foo"), ast.InternedTerm("bar")),
ast.SetTerm(ast.InternedTerm("baz")),
ast.ObjectTerm(ast.Item(ast.InternedTerm("k"), ast.InternedTerm("v"))),
ast.NullTerm(),
ast.InternedBooleanTrue,
),
}
expected := ast.StringTerm(`word 3.141590 more 55 ["foo", "bar"] {"baz"} {"k": "v"} null true`)
for b.Loop() {
if err := builtinSprintf(bctx, operands, eqIter(expected)); err != nil {
b.Fatal(err)
}
}
}
func repeatTerm(t *ast.Term, n int) *ast.Term {
terms := make([]*ast.Term, 0, n)
for range n {
v1/util/performance.go
+52
View File
@@ -114,6 +114,58 @@ func AppendInt(buf []byte, n int) []byte {
return strconv.AppendInt(buf, int64(n), 10)
}
// Atoi is a convenience function for [Atoi64] where an int is preferable to an int64.
// See the documentation of [Atoi64] for details on the performance benefits of this
// function over strconv.Atoi.
func Atoi(s string) (int, bool) {
if i, ok := Atoi64(s); ok {
return int(i), true
}
return 0, false
}
// Atoi64 is an alternative implementation of strconv.Atoi which is slightly faster for the
// (for our use case) common case of a successful conversion, and crucially — *much* faster
// for the failure case, as this function allocates nothing for any given input string, while
// strconv.Atoi performs 1-2 allocations on failure in its error handling. The callers in this
// codebase — most notably ast.Number's Int() and Int64() methods — have no interest in the
// details of the failure, and keeping this allocation free means both methods can be used
// not only for conversion, but as a most efficient "IsInt64" check.
func Atoi64(s string) (int64, bool) {
sLen := len(s)
if sLen > 0 {
negative := s[0] == '-'
if negative || s[0] == '+' {
s = s[1:]
sLen--
}
if sLen == 0 || sLen > 19 {
return 0, false
}
var n int64
for _, ch := range []byte(s) {
ch -= '0'
if ch > 9 {
return 0, false
}
n = n*10 + int64(ch)
}
if !negative && n < 0 {
return 0, false // overflow
}
if negative {
n = -n
if n > 0 {
return 0, false // underflow
}
}
return n, true
}
return 0, false
}
// SplitMap calls fn for each delim-separated part of text and returns a slice of the results.
// Cheaper than calling fn on strings.Split(text, delim), as it avoids allocating an intermediate slice of strings.
func SplitMap[T any](text string, delim string, fn func(string) T) []T {
v1/util/performance_test.go
+77 -1
View File
@@ -67,7 +67,83 @@ func BenchmarkSlicePoolGetPut(b *testing.B) {
}
}
func TestAtoi64(t *testing.T) {
tests := []struct {
input string
expOK bool
expInt int64
}{
{"", false, 0},
{"no", false, 0},
{"02", true, 2},
{"0", true, 0},
{"-0", true, 0},
{"123", true, 123},
{"-123", true, -123},
{"9223372", true, 9223372},
{"8223372036854775807", true, 8223372036854775807},
{"9223372036854775807", true, 9223372036854775807}, // max int64
{"+9223372036854775807", true, 9223372036854775807}, // max int64, leading '+'
{"9223372036854775808", false, 0}, // max int64 + 1
{"-9223372036854775808", true, -9223372036854775808}, // min int64
{"-9223372036854775809", false, 0}, // min int64 - 1
}
for _, test := range tests {
res, ok := Atoi64(test.input)
if ok != test.expOK || res != test.expInt {
t.Errorf("Atoi64(%q) = (%d, %v); expected (%d, %v)", test.input, res, ok, test.expInt, test.expOK)
}
strconvRes, err := strconv.Atoi(test.input)
strconvOK := err == nil
if strconvOK && test.expOK {
if strconvRes != int(test.expInt) {
t.Errorf("strconv.Atoi(%q) = (%d, %v); expected (%d, %v)", test.input, strconvRes, strconvOK, test.expInt, test.expOK)
}
} else {
if strconvOK {
t.Fatalf("strconv.Atoi(%q) = (%d, %v); expected error: %v", test.input, strconvRes, strconvOK, !test.expOK)
}
if test.expOK {
t.Fatalf("strconv.Atoi(%q) error = %v; expected error: %v", test.input, err, !test.expOK)
}
}
}
}
// See testdata/atoi.txt for a performance comparison between Atoi64 and strconv.Atoi
func BenchmarkAtoi64(b *testing.B) {
tests := []string{
"",
"no",
"02",
"0",
"-0",
"123",
"-123",
"9223372",
"8223372036854775807",
"9223372036854775807", // max int64
"+9223372036854775807", // max int64, leading '+'
"9223372036854775808", // max int64 + 1
"-9223372036854775808", // min int64
"-9223372036854775809", // min int64 - 1
}
for _, test := range tests {
b.Run(test, func(b *testing.B) {
for b.Loop() {
// replace with strconv.Atoi for comparison
_, _ = strconv.Atoi(test)
}
})
}
}
func mustAtoi(s string) int {
v, _ := strconv.Atoi(s)
v, _ := Atoi(s)
return v
}
v1/util/testdata/atoi.txt
Vendored
+35
View File
@@ -0,0 +1,35 @@
# Atoi64 vs strconv.Atoi
## Atoi64
BenchmarkAtoi64/#00-16 1000000000 0.9167 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/no-16 1000000000 1.167 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/02-16 573961748 2.107 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/0-16 643013014 1.818 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/-0-16 633210019 1.938 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/123-16 479271106 2.403 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/-123-16 458470455 2.645 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/9223372-16 346216180 3.434 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/8223372036854775807-16 130872100 9.137 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/9223372036854775807-16 131376300 9.159 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/+9223372036854775807-16 124460583 9.627 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/9223372036854775808-16 131100723 9.151 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/-9223372036854775808-16 122350833 9.829 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/-9223372036854775809-16 122665735 9.780 ns/op 0 B/op 0 allocs/op
## strconv.Atoi
BenchmarkAtoi64/#00-16 103776993 11.43 ns/op 48 B/op 1 allocs/op
BenchmarkAtoi64/no-16 72848500 15.40 ns/op 50 B/op 2 allocs/op
BenchmarkAtoi64/02-16 454403812 2.665 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/0-16 496121979 2.404 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/-0-16 451980451 2.642 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/123-16 396382513 3.027 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/-123-16 373580103 3.251 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/9223372-16 302050006 4.098 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/8223372036854775807-16 63019692 19.30 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/9223372036854775807-16 64338274 19.10 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/+9223372036854775807-16 64576740 19.33 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/9223372036854775808-16 35374750 33.59 ns/op 72 B/op 2 allocs/op
BenchmarkAtoi64/-9223372036854775808-16 63774238 19.00 ns/op 0 B/op 0 allocs/op
BenchmarkAtoi64/-9223372036854775809-16 34102534 33.55 ns/op 72 B/op 2 allocs/op