Skip to content

Commit

Permalink
Even less allocs (#7190)
Browse files Browse the repository at this point in the history
**main**
```
BenchmarkLintAllEnabled-10    1	2640715625 ns/op	6385110200 B/op	116296633 allocs/op
```

**pr**
```
BenchmarkLintAllEnabled-10    1	2597179708 ns/op	6183614112 B/op	108421141 allocs/op
```

(I renamed the benchmark, but this is the same as "regal linting itself"
used in the past)

Another 8 million allocations cut off from `regal lint bundle`,
and a whopping 10% improvements to wall clock time!

The most significant improvement is the Equal implementation for
refs, since that is called all over the place. But there are many
other fixes here, and they all contribute something substantial
(and fixes that only have had marginal impact have been left out).

Signed-off-by: Anders Eknert <anders@styra.com>
  • Loading branch information
anderseknert authored Nov 24, 2024
1 parent d3b64d6 commit a60ef72
Show file tree
Hide file tree
Showing 15 changed files with 112 additions and 70 deletions.
10 changes: 5 additions & 5 deletions ast/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -1404,7 +1404,7 @@ var NumbersRangeStep = &Builtin{
var UnitsParse = &Builtin{
Name: "units.parse",
Description: `Converts strings like "10G", "5K", "4M", "1500m", and the like into a number.
This number can be a non-integer, such as 1.5, 0.22, etc. Scientific notation is supported,
This number can be a non-integer, such as 1.5, 0.22, etc. Scientific notation is supported,
allowing values such as "1e-3K" (1) or "2.5e6M" (2.5 million M).
Supports standard metric decimal and binary SI units (e.g., K, Ki, M, Mi, G, Gi, etc.) where
Expand All @@ -1425,11 +1425,11 @@ var UnitsParseBytes = &Builtin{
Name: "units.parse_bytes",
Description: `Converts strings like "10GB", "5K", "4mb", or "1e6KB" into an integer number of bytes.
Supports standard byte units (e.g., KB, KiB, etc.) where KB, MB, GB, and TB are treated as decimal
units, and KiB, MiB, GiB, and TiB are treated as binary units. Scientific notation is supported,
Supports standard byte units (e.g., KB, KiB, etc.) where KB, MB, GB, and TB are treated as decimal
units, and KiB, MiB, GiB, and TiB are treated as binary units. Scientific notation is supported,
enabling values like "1.5e3MB" (1500MB) or "2e6GiB" (2 million GiB).
The bytes symbol (b/B) in the unit is optional; omitting it will yield the same result (e.g., "Mi"
The bytes symbol (b/B) in the unit is optional; omitting it will yield the same result (e.g., "Mi"
and "MiB" are equivalent).`,
Decl: types.NewFunction(
types.Args(
Expand Down Expand Up @@ -3387,7 +3387,7 @@ func (b *Builtin) Ref() Ref {
// IsTargetPos returns true if a variable in the i-th position will be bound by
// evaluating the call expression.
func (b *Builtin) IsTargetPos(i int) bool {
return len(b.Decl.FuncArgs().Args) == i
return b.Decl.Arity() == i
}

func init() {
Expand Down
21 changes: 14 additions & 7 deletions ast/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,16 @@ type baseDocEqIndex struct {
onlyGroundRefs bool
}

var (
equalityRef = Equality.Ref()
equalRef = Equal.Ref()
globMatchRef = GlobMatch.Ref()
internalPrintRef = InternalPrint.Ref()
)

func newBaseDocEqIndex(isVirtual func(Ref) bool) *baseDocEqIndex {
return &baseDocEqIndex{
skipIndexing: NewSet(NewTerm(InternalPrint.Ref())),
skipIndexing: NewSet(NewTerm(internalPrintRef)),
isVirtual: isVirtual,
root: newTrieNodeImpl(),
onlyGroundRefs: true,
Expand Down Expand Up @@ -255,17 +262,17 @@ func (i *refindices) Update(rule *Rule, expr *Expr) {
op := expr.Operator()

switch {
case op.Equal(Equality.Ref()):
case op.Equal(equalityRef):
i.updateEq(rule, expr)

case op.Equal(Equal.Ref()) && len(expr.Operands()) == 2:
case op.Equal(equalRef) && len(expr.Operands()) == 2:
// NOTE(tsandall): if equal() is called with more than two arguments the
// output value is being captured in which case the indexer cannot
// exclude the rule if the equal() call would return false (because the
// false value must still be produced.)
i.updateEq(rule, expr)

case op.Equal(GlobMatch.Ref()) && len(expr.Operands()) == 3:
case op.Equal(globMatchRef) && len(expr.Operands()) == 3:
// NOTE(sr): Same as with equal() above -- 4 operands means the output
// of `glob.match` is captured and the rule can thus not be excluded.
i.updateGlobMatch(rule, expr)
Expand Down Expand Up @@ -354,7 +361,7 @@ func (i *refindices) updateGlobMatch(rule *Rule, expr *Expr) {
if ref == nil {
for j, arg := range args {
if arg.Equal(match) {
ref = Ref{FunctionArgRootDocument, IntNumberTerm(j)}
ref = Ref{FunctionArgRootDocument, InternedIntNumberTerm(j)}
}
}
}
Expand Down Expand Up @@ -432,7 +439,7 @@ func (tr *trieTraversalResult) Add(t *trieNode) {
tr.unordered[root] = append(nodes, node)
}
if t.values != nil {
t.values.Foreach(func(v *Term) { tr.values.Add(v) })
t.values.Foreach(tr.values.Add)
}
}

Expand Down Expand Up @@ -764,7 +771,7 @@ func eqOperandsToRefAndValue(isVirtual func(Ref) bool, args []*Term, a, b *Term)
for i, arg := range args {
if arg.Value.Compare(v) == 0 {
if bval, ok := indexValue(b); ok {
return &refindex{Ref: Ref{FunctionArgRootDocument, IntNumberTerm(i)}, Value: bval}, true
return &refindex{Ref: Ref{FunctionArgRootDocument, InternedIntNumberTerm(i)}, Value: bval}, true
}
}
}
Expand Down
34 changes: 21 additions & 13 deletions ast/internal/scanner/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io"
"unicode"
"unicode/utf8"
"unsafe"

"github.com/open-policy-agent/opa/ast/internal/tokens"
)
Expand All @@ -18,31 +19,31 @@ const bom = 0xFEFF
// Scanner is used to tokenize an input stream of
// Rego source code.
type Scanner struct {
keywords map[string]tokens.Token
bs []byte
errors []Error
tabs []int
offset int
row int
col int
bs []byte
curr rune
width int
errors []Error
keywords map[string]tokens.Token
tabs []int
curr rune
regoV1Compatible bool
}

// Error represents a scanner error.
type Error struct {
Pos Position
Message string
Pos Position
}

// Position represents a point in the scanned source code.
type Position struct {
Tabs []int // positions of any tabs preceding Col
Offset int // start offset in bytes
End int // end offset in bytes
Row int // line number computed in bytes
Col int // column number computed in bytes
Tabs []int // positions of any tabs preceding Col
}

// New returns an initialized scanner that will scan
Expand Down Expand Up @@ -270,7 +271,8 @@ func (s *Scanner) scanIdentifier() string {
for isLetter(s.curr) || isDigit(s.curr) {
s.next()
}
return string(s.bs[start : s.offset-1])

return byteSliceToString(s.bs[start : s.offset-1])
}

func (s *Scanner) scanNumber() string {
Expand Down Expand Up @@ -321,7 +323,7 @@ func (s *Scanner) scanNumber() string {
}
}

return string(s.bs[start : s.offset-1])
return byteSliceToString(s.bs[start : s.offset-1])
}

func (s *Scanner) scanString() string {
Expand Down Expand Up @@ -355,7 +357,7 @@ func (s *Scanner) scanString() string {
}
}

return string(s.bs[start : s.offset-1])
return byteSliceToString(s.bs[start : s.offset-1])
}

func (s *Scanner) scanRawString() string {
Expand All @@ -370,7 +372,8 @@ func (s *Scanner) scanRawString() string {
break
}
}
return string(s.bs[start : s.offset-1])

return byteSliceToString(s.bs[start : s.offset-1])
}

func (s *Scanner) scanComment() string {
Expand All @@ -383,7 +386,8 @@ func (s *Scanner) scanComment() string {
if s.offset > 1 && s.bs[s.offset-2] == '\r' {
end = end - 1
}
return string(s.bs[start:end])

return byteSliceToString(s.bs[start:end])
}

func (s *Scanner) next() {
Expand Down Expand Up @@ -413,7 +417,7 @@ func (s *Scanner) next() {
if s.curr == '\n' {
s.row++
s.col = 0
s.tabs = []int{}
s.tabs = s.tabs[:0]
} else {
s.col++
if s.curr == '\t' {
Expand Down Expand Up @@ -453,3 +457,7 @@ func (s *Scanner) error(reason string) {
Col: s.col,
}, Message: reason})
}

func byteSliceToString(bs []byte) string {
return unsafe.String(unsafe.SliceData(bs), len(bs))
}
2 changes: 1 addition & 1 deletion ast/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (vs *ValueMap) Equal(other *ValueMap) bool {
return other == nil || other.Len() == 0
}
if other == nil {
return vs == nil || vs.Len() == 0
return vs.Len() == 0
}
return vs.hashMap.Equal(other.hashMap)
}
Expand Down
25 changes: 18 additions & 7 deletions ast/term.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,8 @@ func (term *Term) Equal(other *Term) bool {
return v.Equal(other.Value)
case Var:
return v.Equal(other.Value)
case Ref:
return v.Equal(other.Value)
}

return term.Value.Compare(other.Value) == 0
Expand Down Expand Up @@ -992,7 +994,20 @@ func (ref Ref) Copy() Ref {

// Equal returns true if ref is equal to other.
func (ref Ref) Equal(other Value) bool {
return Compare(ref, other) == 0
switch o := other.(type) {
case Ref:
if len(ref) == len(o) {
for i := range ref {
if !ref[i].Equal(o[i]) {
return false
}
}

return true
}
}

return false
}

// Compare compares ref to other, return <0, 0, or >0 if it is less than, equal to,
Expand Down Expand Up @@ -1578,12 +1593,8 @@ func (s *set) Intersect(other Set) Set {
// Union returns the set containing all elements of s and other.
func (s *set) Union(other Set) Set {
r := NewSet()
s.Foreach(func(x *Term) {
r.Add(x)
})
other.Foreach(func(x *Term) {
r.Add(x)
})
s.Foreach(r.Add)
other.Foreach(r.Add)
return r
}

Expand Down
4 changes: 2 additions & 2 deletions builtin_metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -21380,7 +21380,7 @@
"v0.70.0",
"edge"
],
"description": "Converts strings like \"10G\", \"5K\", \"4M\", \"1500m\", and the like into a number.\nThis number can be a non-integer, such as 1.5, 0.22, etc. Scientific notation is supported, \nallowing values such as \"1e-3K\" (1) or \"2.5e6M\" (2.5 million M).\n\nSupports standard metric decimal and binary SI units (e.g., K, Ki, M, Mi, G, Gi, etc.) where\nm, K, M, G, T, P, and E are treated as decimal units and Ki, Mi, Gi, Ti, Pi, and Ei are treated as\nbinary units.\n\nNote that 'm' and 'M' are case-sensitive to allow distinguishing between \"milli\" and \"mega\" units\nrespectively. Other units are case-insensitive.",
"description": "Converts strings like \"10G\", \"5K\", \"4M\", \"1500m\", and the like into a number.\nThis number can be a non-integer, such as 1.5, 0.22, etc. Scientific notation is supported,\nallowing values such as \"1e-3K\" (1) or \"2.5e6M\" (2.5 million M).\n\nSupports standard metric decimal and binary SI units (e.g., K, Ki, M, Mi, G, Gi, etc.) where\nm, K, M, G, T, P, and E are treated as decimal units and Ki, Mi, Gi, Ti, Pi, and Ei are treated as\nbinary units.\n\nNote that 'm' and 'M' are case-sensitive to allow distinguishing between \"milli\" and \"mega\" units\nrespectively. Other units are case-insensitive.",
"introduced": "v0.41.0",
"result": {
"description": "the parsed number",
Expand Down Expand Up @@ -21508,7 +21508,7 @@
"v0.70.0",
"edge"
],
"description": "Converts strings like \"10GB\", \"5K\", \"4mb\", or \"1e6KB\" into an integer number of bytes.\n\nSupports standard byte units (e.g., KB, KiB, etc.) where KB, MB, GB, and TB are treated as decimal \nunits, and KiB, MiB, GiB, and TiB are treated as binary units. Scientific notation is supported, \nenabling values like \"1.5e3MB\" (1500MB) or \"2e6GiB\" (2 million GiB).\n\nThe bytes symbol (b/B) in the unit is optional; omitting it will yield the same result (e.g., \"Mi\" \nand \"MiB\" are equivalent).",
"description": "Converts strings like \"10GB\", \"5K\", \"4mb\", or \"1e6KB\" into an integer number of bytes.\n\nSupports standard byte units (e.g., KB, KiB, etc.) where KB, MB, GB, and TB are treated as decimal\nunits, and KiB, MiB, GiB, and TiB are treated as binary units. Scientific notation is supported,\nenabling values like \"1.5e3MB\" (1500MB) or \"2e6GiB\" (2 million GiB).\n\nThe bytes symbol (b/B) in the unit is optional; omitting it will yield the same result (e.g., \"Mi\"\nand \"MiB\" are equivalent).",
"introduced": "v0.17.0",
"result": {
"description": "the parsed number",
Expand Down
14 changes: 7 additions & 7 deletions format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ func (w *writer) writeFunctionCall(expr *ast.Expr, comments []*ast.Comment) []*a
return w.writeFunctionCallPlain(terms, comments)
}

numDeclArgs := len(bi.Decl.Args())
numDeclArgs := bi.Decl.Arity()
numCallArgs := len(terms) - 1

switch numCallArgs {
Expand Down Expand Up @@ -918,7 +918,7 @@ func (w *writer) writeCall(parens bool, x ast.Call, loc *ast.Location, comments
// NOTE(Trolloldem): writeCall is only invoked when the function call is a term
// of another function. The only valid arity is the one of the
// built-in function
if len(bi.Decl.Args()) != len(x)-1 {
if bi.Decl.Arity() != len(x)-1 {
w.errs = append(w.errs, ArityFormatMismatchError(x[1:], x[0].String(), loc, bi.Decl))
return comments
}
Expand All @@ -935,10 +935,10 @@ func (w *writer) writeCall(parens bool, x ast.Call, loc *ast.Location, comments

func (w *writer) writeInOperator(parens bool, operands []*ast.Term, comments []*ast.Comment, loc *ast.Location, f *types.Function) []*ast.Comment {

if len(operands) != len(f.Args()) {
if len(operands) != f.Arity() {
// The number of operands does not math the arity of the `in` operator
operator := ast.Member.Name
if len(f.Args()) == 3 {
if f.Arity() == 3 {
operator = ast.MemberWithKey.Name
}
w.errs = append(w.errs, ArityFormatMismatchError(operands, operator, loc, f))
Expand Down Expand Up @@ -1603,9 +1603,9 @@ type ArityFormatErrDetail struct {

// arityMismatchError but for `fmt` checks since the compiler has not run yet.
func ArityFormatMismatchError(operands []*ast.Term, operator string, loc *ast.Location, f *types.Function) *ast.Error {
want := make([]string, len(f.Args()))
for i := range f.Args() {
want[i] = types.Sprint(f.Args()[i])
want := make([]string, f.Arity())
for i, arg := range f.Args() {
want[i] = types.Sprint(arg)
}

have := make([]string, len(operands))
Expand Down
2 changes: 1 addition & 1 deletion internal/edittree/edittree.go
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ func (e *EditTree) Unfold(path ast.Ref) (*EditTree, error) {
// Extend the tree if key is present. Error otherwise.
if v, err := x.Find(ast.Ref{ast.InternedIntNumberTerm(idx)}); err == nil {
// TODO: Consider a more efficient "Replace" function that special-cases this for arrays instead?
_, err := e.Delete(ast.IntNumberTerm(idx))
_, err := e.Delete(ast.InternedIntNumberTerm(idx))
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/planner/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@ func (p *Planner) planExprCall(e *ast.Expr, iter planiter) error {
args = p.defaultOperands()
} else if decl, ok := p.decls[operator]; ok {
relation = decl.Relation
arity = len(decl.Decl.Args())
arity = decl.Decl.Arity()
void = decl.Decl.Result() == nil
name = operator
p.externs[operator] = decl
Expand Down
2 changes: 1 addition & 1 deletion internal/strvals/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var ErrNotList = errors.New("not a list")

// MaxIndex is the maximum index that will be allowed by setIndex.
// The default value 65536 = 1024 * 64
var MaxIndex = 65536
const MaxIndex = 65536

// ToYAML takes a string of arguments and converts to a YAML document.
func ToYAML(s string) (string, error) {
Expand Down
2 changes: 1 addition & 1 deletion rego/rego.go
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ func memoize(decl *Function, bctx BuiltinContext, terms []*ast.Term, ifEmpty fun
// The term slice _may_ include an output term depending on how the caller
// referred to the built-in function. Only use the arguments as the cache
// key. Unification ensures we don't get false positive matches.
for i := 0; i < len(decl.Decl.Args()); i++ {
for i := 0; i < decl.Decl.Arity(); i++ {
if _, err := b.WriteString(terms[i].String()); err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit a60ef72

Please sign in to comment.