mirror of
https://github.com/open-policy-agent/opa.git
synced 2026-08-12 19:32:48 -06:00
perf: replace regex implementation in IsVarCompatibleString
The allowed pattern is simple enough to check byte-for-byte, and doing so is much faster, as the benchmarks clearly demonstrate. While it isnt't a bottleneck by any means, this function is called often enough (once for every term in any ref serialized) that optimizing its implementation is warranted, IMO. Signed-off-by: Anders Eknert <anders.eknert@apple.com>
This commit is contained in:
committed by
Stephan Renatus
parent
d10b4f59b4
commit
4d1dfc4f7c
+30
-4
@@ -12,7 +12,6 @@ import (
|
||||
"io"
|
||||
"math"
|
||||
"net/url"
|
||||
"regexp"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
@@ -29,8 +28,6 @@ var (
|
||||
NullValue Value = Null{}
|
||||
|
||||
errFindNotFound = errors.New("find: not found")
|
||||
|
||||
varRegexp = regexp.MustCompile("^[[:alpha:]_][[:alpha:][:digit:]_]*$")
|
||||
)
|
||||
|
||||
// Location records a position in source code.
|
||||
@@ -1340,8 +1337,37 @@ func (ref Ref) Ptr() (string, error) {
|
||||
return buf.String(), nil
|
||||
}
|
||||
|
||||
// IsVarCompatibleString returns true if s is a valid variable name. String s is a valid variable
|
||||
// name if it starts with a letter (a-z or A-Z) or underscore (_) and is followed by
|
||||
// letters (a-z or A-Z), digits (0-9), and underscores.
|
||||
func IsVarCompatibleString(s string) bool {
|
||||
return varRegexp.MatchString(s)
|
||||
l := len(s)
|
||||
if l == 0 {
|
||||
return false
|
||||
}
|
||||
// not exactly easy on the eyes, but often orders of magnitude faster
|
||||
// than using a compiled regex (see benchmarks in term_bench_test.go)
|
||||
is_letter := func(c byte) bool {
|
||||
return (c > 96 && c < 123) || (c > 64 && c < 91)
|
||||
}
|
||||
is_digit := func(c byte) bool {
|
||||
return c > 47 && c < 58
|
||||
}
|
||||
|
||||
// first character must be a letter or underscore
|
||||
c := s[0]
|
||||
if !(is_letter(c) || c == 95) {
|
||||
return false
|
||||
}
|
||||
|
||||
// remaining characters must be letters, digits, or underscores
|
||||
for i := 1; i < l; i++ {
|
||||
if c = s[i]; !(is_letter(c) || is_digit(c) || c == 95) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
func (ref Ref) String() string {
|
||||
|
||||
+19
-10
@@ -678,7 +678,6 @@ func BenchmarkSetString(b *testing.B) {
|
||||
}
|
||||
|
||||
func BenchmarkSetMarshalJSON(b *testing.B) {
|
||||
var err error
|
||||
sizes := []int{5, 50, 500, 5000, 50000}
|
||||
|
||||
for _, n := range sizes {
|
||||
@@ -689,7 +688,7 @@ func BenchmarkSetMarshalJSON(b *testing.B) {
|
||||
}
|
||||
|
||||
b.Run("json.Marshal", func(b *testing.B) {
|
||||
b.ResetTimer()
|
||||
var err error
|
||||
for b.Loop() {
|
||||
bs, err = json.Marshal(set)
|
||||
if err != nil {
|
||||
@@ -701,16 +700,26 @@ func BenchmarkSetMarshalJSON(b *testing.B) {
|
||||
}
|
||||
}
|
||||
|
||||
// Rationale for using byte-by-byte check vs regex in IsVarCompatibleString
|
||||
// ---------------------------------------------------------------------------
|
||||
// string byte check regex
|
||||
// ---------------------------------------- --------------- ------------
|
||||
// "" 1.60 ns/op 2.14 ns/op
|
||||
// "5heel" 1.57 ns/op 17.21 ns/op
|
||||
// "__really_long_variable_name_1234567890" 13.84 ns/op 342.10 ns/op
|
||||
// "_ello_" 3.07 ns/op 66.35 ns/op
|
||||
// "h_llo" 1.61 ns/op 23.86 ns/op
|
||||
// "hello" 2.35 ns/op 59.42 ns/op
|
||||
// "incompatible_last_char!" 7.85 ns/op 208.60 ns/op
|
||||
func BenchmarkIsVarCompatibleString(b *testing.B) {
|
||||
tests := map[string]bool{
|
||||
"hello": true,
|
||||
"5heel": false,
|
||||
"h\nllo": false,
|
||||
"h\tllo": false,
|
||||
"h\x00llo": false,
|
||||
"h\"llo": false,
|
||||
"h\\llo": false,
|
||||
"": false,
|
||||
"hello": true,
|
||||
"_ello_": true,
|
||||
"5heel": false,
|
||||
"h\nllo": false,
|
||||
"": false,
|
||||
"incompatible_last_char!": false,
|
||||
"__really_long_variable_name_1234567890": true,
|
||||
}
|
||||
|
||||
for _, name := range util.KeysSorted(tests) {
|
||||
|
||||
+1
-3
@@ -9,7 +9,6 @@ import (
|
||||
"bytes"
|
||||
"errors"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"slices"
|
||||
"sort"
|
||||
"strings"
|
||||
@@ -29,7 +28,6 @@ const defaultLocationFile = "__format_default__"
|
||||
var (
|
||||
expandedConst = ast.NewBody(ast.NewExpr(ast.InternedTerm(true)))
|
||||
commentsSlicePool = util.NewSlicePool[*ast.Comment](50)
|
||||
varRegexp = regexp.MustCompile("^[[:alpha:]_][[:alpha:][:digit:]_]*$")
|
||||
)
|
||||
|
||||
// Opts lets you control the code formatting via `AstWithOpts()`.
|
||||
@@ -1441,7 +1439,7 @@ func (w *writer) writeRefStringPath(s ast.String, l *ast.Location) {
|
||||
}
|
||||
|
||||
func (w *writer) shouldBracketRefTerm(s string, l *ast.Location) bool {
|
||||
if !varRegexp.MatchString(s) {
|
||||
if !ast.IsVarCompatibleString(s) {
|
||||
return true
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user