diff --git a/.golangci.yaml b/.golangci.yaml index 66897f1ac12..ab61a0ca09d 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -6,6 +6,7 @@ linters: enable: - copyloopvar - errcheck + - forbidigo - gocritic - govet - ineffassign @@ -26,6 +27,10 @@ linters: exclude-functions: - github.com/open-policy-agent/opa/v1/util.WriteAppender - github.com/open-policy-agent/opa/v1/util.WriteInt + forbidigo: + forbid: + - pattern: '^sort\.[A-Z][a-zA-Z]+$' + message: "Prefer the more performant sort/search functions in the slices package" gocritic: enabled-checks: # NOTE that these are rules enabled in addition to the default set diff --git a/cmd/capabilities_jsonv2_test.go b/cmd/capabilities_jsonv2_test.go index b5397e00da4..7b626e1e45b 100644 --- a/cmd/capabilities_jsonv2_test.go +++ b/cmd/capabilities_jsonv2_test.go @@ -10,7 +10,6 @@ import ( "bytes" "path" "slices" - "sort" "testing" "github.com/google/go-cmp/cmp" @@ -189,8 +188,8 @@ func TestCapabilitiesCurrent(t *testing.T) { for _, tc := range tests { t.Run(tc.note, func(t *testing.T) { // These are sorted in the output - sort.Strings(tc.expFutureKeywords) - sort.Strings(tc.expFeatures) + slices.Sort(tc.expFutureKeywords) + slices.Sort(tc.expFeatures) params := capabilitiesParams{ showCurrent: true, diff --git a/cmd/capabilities_test.go b/cmd/capabilities_test.go index 4dc715f414e..bcb50b2bf7f 100644 --- a/cmd/capabilities_test.go +++ b/cmd/capabilities_test.go @@ -10,7 +10,6 @@ import ( "bytes" "path" "slices" - "sort" "testing" "github.com/google/go-cmp/cmp" @@ -189,8 +188,8 @@ func TestCapabilitiesCurrent(t *testing.T) { for _, tc := range tests { t.Run(tc.note, func(t *testing.T) { // These are sorted in the output - sort.Strings(tc.expFutureKeywords) - sort.Strings(tc.expFeatures) + slices.Sort(tc.expFutureKeywords) + slices.Sort(tc.expFeatures) params := capabilitiesParams{ showCurrent: true, diff --git a/cmd/inspect.go b/cmd/inspect.go index 6187d26ab98..262ee042078 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -10,7 +10,6 @@ import ( "fmt" "io" "os" - "sort" "strconv" "strings" @@ -198,8 +197,7 @@ func populateManifest(out io.Writer, m *bundle.Manifest) error { lines = append(lines, []string{"Roots", truncateFileName(roots[0])}) } } else { - sort.Strings(roots) - for _, root := range roots { + for _, root := range util.Sorted(roots) { lines = append(lines, []string{"Roots", truncateFileName(root)}) } } diff --git a/cmd/version_test.go b/cmd/version_test.go index f2e2dc5b25e..34e61c93740 100644 --- a/cmd/version_test.go +++ b/cmd/version_test.go @@ -12,7 +12,7 @@ import ( "net/http" "net/http/httptest" "runtime" - "sort" + "slices" "strings" "testing" @@ -102,16 +102,16 @@ func expectOutputKeys(t *testing.T, stdout string, expectedKeys []string) { gotKeys := make([]string, 0, len(lines)) for _, line := range lines { - gotKeys = append(gotKeys, strings.Split(line, ":")[0]) + key, _, _ := strings.Cut(line, ":") + gotKeys = append(gotKeys, key) } - sort.Strings(expectedKeys) - sort.Strings(gotKeys) - + slices.Sort(expectedKeys) if len(expectedKeys) != len(gotKeys) { t.Fatalf("expected %v but got %v", expectedKeys, gotKeys) } + slices.Sort(gotKeys) for i, got := range gotKeys { if expectedKeys[i] != got { t.Fatalf("expected %v but got %v", expectedKeys, gotKeys) diff --git a/e2e/proto/protoschemacheck/protoschemacheck.go b/e2e/proto/protoschemacheck/protoschemacheck.go index de784f7b895..85035ef4d67 100644 --- a/e2e/proto/protoschemacheck/protoschemacheck.go +++ b/e2e/proto/protoschemacheck/protoschemacheck.go @@ -14,12 +14,12 @@ import ( "maps" "reflect" "slices" - "sort" "strings" "testing" "time" "github.com/bufbuild/protocompile" + "github.com/open-policy-agent/opa/v1/util" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -476,8 +476,7 @@ func checkOneof(t *testing.T, declared map[string]protoreflect.MessageDescriptor orphanCases = append(orphanCases, caseName) } } - sort.Strings(orphanCases) - for _, caseName := range orphanCases { + for _, caseName := range util.Sorted(orphanCases) { f := caseFields[caseName] t.Errorf("%s.%s: proto case %q (number %d) has no corresponding discriminator in DiscriminatorToCase; either remove it (and `reserved %d` the number) or extend the spec", o.MessageName, o.OneofName, caseName, f.Number(), f.Number()) } diff --git a/internal/cmd/genplanschema/main.go b/internal/cmd/genplanschema/main.go index 410b6c6f31c..c4de144b4fa 100644 --- a/internal/cmd/genplanschema/main.go +++ b/internal/cmd/genplanschema/main.go @@ -11,10 +11,10 @@ import ( "log" "os" "reflect" - "sort" "github.com/open-policy-agent/opa/internal/genjsonschema" "github.com/open-policy-agent/opa/v1/ir" + "github.com/open-policy-agent/opa/v1/util" ) func main() { @@ -132,7 +132,7 @@ func addValUnion(b *genjsonschema.Builder) (string, error) { return b.DefRef(name), nil } vals := ir.ValKinds() - kinds := sortedKeys(vals) + kinds := util.KeysSorted(vals) branches := make([]any, 0, len(kinds)) for _, kind := range kinds { valueSchema, err := b.ReflectType(reflect.TypeOf(vals[kind])) @@ -183,7 +183,7 @@ func addStmtUnion(b *genjsonschema.Builder) (string, error) { } stmts := ir.StmtKinds() - kinds := sortedKeys(stmts) + kinds := util.KeysSorted(stmts) branches := make([]any, 0, len(kinds)) for _, kind := range kinds { bodyRef, err := b.AddStruct(reflect.TypeOf(stmts[kind])) @@ -227,14 +227,3 @@ func makeNumberRefStmtSchema() genjsonschema.OrderedMap { "additionalProperties", false, ) } - -// sortedKeys returns the keys of m in lexicographic order so the polymorphic -// Stmt/Val unions render their branches in a byte-stable order. -func sortedKeys[V any](m map[string]V) []string { - keys := make([]string, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - sort.Strings(keys) - return keys -} diff --git a/internal/genjsonschema/genjsonschema.go b/internal/genjsonschema/genjsonschema.go index 5ee1932aac5..e383580af5f 100644 --- a/internal/genjsonschema/genjsonschema.go +++ b/internal/genjsonschema/genjsonschema.go @@ -16,7 +16,6 @@ import ( "fmt" "reflect" "slices" - "sort" "strings" ) @@ -71,7 +70,7 @@ func (b *Builder) DefsOrdered() OrderedMap { for n := range b.defs { names = append(names, n) } - sort.Strings(names) + slices.Sort(names) out := make(OrderedMap, 0, len(names)) for _, n := range names { out = append(out, Entry{n, b.defs[n]}) @@ -159,7 +158,7 @@ func (b *Builder) reflectStructBody(t reflect.Type) (OrderedMap, error) { return nil, err } - sort.Strings(required) + slices.Sort(required) out := OrderedMap{ {"type", "object"}, @@ -221,7 +220,7 @@ func (b *Builder) collectFields(t reflect.Type, properties *OrderedMap, required }) } - sort.Slice(fields, func(i, j int) bool { return fields[i].name < fields[j].name }) + slices.SortFunc(fields, func(a, b pendingField) int { return strings.Compare(a.name, b.name) }) for _, f := range fields { *properties = append(*properties, Entry{f.name, f.schema}) if f.required { diff --git a/internal/lcss/qsufsort.go b/internal/lcss/qsufsort.go index 61c51968869..cc0d26c6e51 100644 --- a/internal/lcss/qsufsort.go +++ b/internal/lcss/qsufsort.go @@ -55,7 +55,7 @@ func qsufsort(data []byte) []int { } pk := inv[s] + 1 // pk-1 is last position of unsorted group sufSortable.sa = sa[pi:pk] - sort.Sort(sufSortable) + sort.Sort(sufSortable) //nolint:forbidigo sufSortable.updateGroups(pi) pi = pk // next group } diff --git a/internal/planner/planner.go b/internal/planner/planner.go index a7d62542c24..2a24d28ad15 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -6,10 +6,12 @@ package planner import ( + "cmp" "errors" "fmt" "io" - "sort" + "slices" + "strings" "github.com/open-policy-agent/opa/internal/debug" "github.com/open-policy-agent/opa/v1/ast" @@ -164,12 +166,12 @@ func (p *Planner) planRules(rules []*ast.Rule) (string, error) { // We sort rules, first by ref length, and then using the // Ref.Compare method to break ties. This yields a stable // sorting order for the slice of rules to be planned. - sort.Slice(rules, func(i, j int) bool { - li, lj := len(rules[i].Ref()), len(rules[j].Ref()) - if li != lj { - return li > lj + slices.SortFunc(rules, func(a, b *ast.Rule) int { + aRef, bRef := a.Ref(), b.Ref() + if c := cmp.Compare(len(aRef), len(bRef)); c != 0 { + return -c } - return rules[i].Ref().Compare(rules[j].Ref()) < 0 + return aRef.Compare(bRef) }) // We know the rules that are closer to the root (shorter static path) are ordered first. @@ -2441,15 +2443,14 @@ func (p *Planner) planTermSliceRec(terms []*ast.Term, locals []ir.Operand, index } func (p *Planner) planExterns() error { - p.policy.Static.BuiltinFuncs = make([]*ir.BuiltinFunc, 0, len(p.externs)) for name, decl := range p.externs { p.policy.Static.BuiltinFuncs = append(p.policy.Static.BuiltinFuncs, &ir.BuiltinFunc{Name: name, Decl: decl.Decl}) } - sort.Slice(p.policy.Static.BuiltinFuncs, func(i, j int) bool { - return p.policy.Static.BuiltinFuncs[i].Name < p.policy.Static.BuiltinFuncs[j].Name + slices.SortFunc(p.policy.Static.BuiltinFuncs, func(a, b *ir.BuiltinFunc) int { + return strings.Compare(a.Name, b.Name) }) return nil diff --git a/internal/planner/rules.go b/internal/planner/rules.go index ed4da8571b8..8b6871269c7 100644 --- a/internal/planner/rules.go +++ b/internal/planner/rules.go @@ -2,7 +2,6 @@ package planner import ( "fmt" - "sort" "github.com/open-policy-agent/opa/v1/ast" "github.com/open-policy-agent/opa/v1/util" @@ -242,10 +241,7 @@ func (t *ruletrie) Children() []ast.Value { sorted = append(sorted, key) } } - sort.Slice(sorted, func(i, j int) bool { - return sorted[i].Compare(sorted[j]) < 0 - }) - return sorted + return util.SortedFunc(sorted, ast.Value.Compare) } func (t *ruletrie) Get(k ast.Value) *ruletrie { diff --git a/internal/presentation/presentation.go b/internal/presentation/presentation.go index 2fc00ebc55d..1b3d7bbec3c 100644 --- a/internal/presentation/presentation.go +++ b/internal/presentation/presentation.go @@ -12,7 +12,6 @@ import ( "fmt" "io" "slices" - "sort" "strconv" "strings" "time" @@ -110,13 +109,8 @@ func (o DepAnalysisOutput) Pretty(w io.Writer) error { } func (o DepAnalysisOutput) sort() { - sort.Slice(o.Base, func(i, j int) bool { - return o.Base[i].Compare(o.Base[j]) < 0 - }) - - sort.Slice(o.Virtual, func(i, j int) bool { - return o.Virtual[i].Compare(o.Virtual[j]) < 0 - }) + slices.SortFunc(o.Base, ast.RefCompare) + slices.SortFunc(o.Virtual, ast.RefCompare) } // Output contains the result of evaluation to be presented. @@ -752,8 +746,8 @@ func populateTableAggregatedMetrics(ms map[string]any, table *tablewriter.Table, } func sortMetricRows(data [][]string) { - sort.Slice(data, func(i, j int) bool { - return data[i][0] < data[j][0] + slices.SortFunc(data, func(a, b []string) int { + return strings.Compare(a[0], b[0]) }) } @@ -763,16 +757,6 @@ type resultKey struct { exprText string } -func resultKeyLess(a, b resultKey) bool { - if a.varName != "" { - if b.varName == "" { - return true - } - return a.varName < b.varName - } - return a.exprIndex < b.exprIndex -} - func (rk resultKey) string() string { if rk.varName != "" { return rk.varName @@ -791,9 +775,7 @@ func generateResultKeys(rs rego.ResultSet) []resultKey { keys := []resultKey{} if len(rs) != 0 { for k := range rs[0].Bindings { - keys = append(keys, resultKey{ - varName: k, - }) + keys = append(keys, resultKey{varName: k}) } for i, expr := range rs[0].Expressions { @@ -805,8 +787,11 @@ func generateResultKeys(rs rego.ResultSet) []resultKey { } } - sort.Slice(keys, func(i, j int) bool { - return resultKeyLess(keys[i], keys[j]) + slices.SortFunc(keys, func(a, b resultKey) int { + if c := strings.Compare(a.varName, b.varName); c != 0 { + return c + } + return a.exprIndex - b.exprIndex }) } return keys diff --git a/internal/providers/aws/signing_v4a.go b/internal/providers/aws/signing_v4a.go index db20eddc9d6..dcf26b4038a 100644 --- a/internal/providers/aws/signing_v4a.go +++ b/internal/providers/aws/signing_v4a.go @@ -16,7 +16,7 @@ import ( "math/big" "net/http" "net/url" - "sort" + "slices" "strconv" "strings" "sync" @@ -211,7 +211,7 @@ func (s *httpSigner) Build() (signedRequest, error) { // Sort Each Query Key's Values for key := range query { - sort.Strings(query[key]) + slices.Sort(query[key]) } v4Internal.SanitizeHostForHeader(req) @@ -319,7 +319,7 @@ func (*httpSigner) buildCanonicalHeaders(host string, rule v4Internal.Rule, head headers = append(headers, lowerCaseKey) signed[lowerCaseKey] = v } - sort.Strings(headers) + slices.Sort(headers) signedHeaders = strings.Join(headers, ";") diff --git a/v1/ast/capabilities.go b/v1/ast/capabilities.go index 0c37dfb1fef..24b2caf509a 100644 --- a/v1/ast/capabilities.go +++ b/v1/ast/capabilities.go @@ -12,7 +12,6 @@ import ( "io" "os" "slices" - "sort" "strings" "sync" @@ -188,8 +187,8 @@ func CapabilitiesForThisVersion(opts ...CapabilitiesOption) *Capabilities { copy(f.Features, Features) } - sort.Strings(f.FutureKeywords) - sort.Strings(f.Features) + slices.Sort(f.FutureKeywords) + slices.Sort(f.Features) return f } @@ -306,14 +305,14 @@ func (c *Capabilities) ContainsFutureKeyword(kw string) bool { // addBuiltinSorted inserts a built-in into c in sorted order. An existing built-in with the same name // will be overwritten. func (c *Capabilities) addBuiltinSorted(bi *Builtin) { - i := sort.Search(len(c.Builtins), func(x int) bool { - return c.Builtins[x].Name >= bi.Name - }) - if i < len(c.Builtins) && bi.Name == c.Builtins[i].Name { - c.Builtins[i] = bi - return + i, found := slices.BinarySearchFunc(c.Builtins, bi, cmpBuiltinName) + if !found { + c.Builtins = append(c.Builtins, nil) + copy(c.Builtins[i+1:], c.Builtins[i:]) } - c.Builtins = append(c.Builtins, nil) - copy(c.Builtins[i+1:], c.Builtins[i:]) c.Builtins[i] = bi } + +func cmpBuiltinName(a, b *Builtin) int { + return strings.Compare(a.Name, b.Name) +} diff --git a/v1/ast/compile.go b/v1/ast/compile.go index a232dd1f722..675b8695f46 100644 --- a/v1/ast/compile.go +++ b/v1/ast/compile.go @@ -11,7 +11,6 @@ import ( "io" "maps" "slices" - "sort" "strings" "sync" @@ -672,7 +671,7 @@ func (c *Compiler) Compile(modules map[string]*Module) { c.init() c.Modules = make(map[string]*Module, len(modules)) - c.sorted = make([]string, 0, len(modules)) + c.sorted = util.KeysSorted(modules) if c.keepModules { c.parsedModules = make(map[string]*Module, len(modules)) @@ -682,14 +681,11 @@ func (c *Compiler) Compile(modules map[string]*Module) { for k, v := range modules { c.Modules[k] = v.Copy() - c.sorted = append(c.sorted, k) if c.parsedModules != nil { c.parsedModules[k] = v } } - sort.Strings(c.sorted) - c.compile() } @@ -2246,7 +2242,6 @@ func (c *Compiler) resolveAllRefs() { } if c.moduleLoader != nil { - parsed, err := c.moduleLoader(c.Modules) if err != nil { c.err(newErrorString(CompileErr, nil, err.Error())) @@ -2265,7 +2260,7 @@ func (c *Compiler) resolveAllRefs() { } } - sort.Strings(c.sorted) + slices.Sort(c.sorted) c.resolveAllRefs() } } @@ -4844,33 +4839,33 @@ func sortGraphNodes(nodes []util.T) { }) } -func (sort *graphSort) Marked(node util.T) bool { - _, marked := sort.marked[node] +func (gs *graphSort) Marked(node util.T) bool { + _, marked := gs.marked[node] return marked } -func (sort *graphSort) Visit(node util.T) (ok bool) { - if _, ok := sort.temp[node]; ok { +func (gs *graphSort) Visit(node util.T) (ok bool) { + if _, ok := gs.temp[node]; ok { return false } - if sort.Marked(node) { + if gs.Marked(node) { return true } - sort.temp[node] = struct{}{} - deps := sort.deps(node) + gs.temp[node] = struct{}{} + deps := gs.deps(node) depList := make([]util.T, 0, len(deps)) for other := range deps { depList = append(depList, other) } sortGraphNodes(depList) for _, other := range depList { - if !sort.Visit(other) { + if !gs.Visit(other) { return false } } - sort.marked[node] = struct{}{} - delete(sort.temp, node) - sort.sorted = append(sort.sorted, node) + gs.marked[node] = struct{}{} + delete(gs.temp, node) + gs.sorted = append(gs.sorted, node) return true } diff --git a/v1/ast/compile_test.go b/v1/ast/compile_test.go index 8ebe867cb1a..a6f3bc2e6cd 100644 --- a/v1/ast/compile_test.go +++ b/v1/ast/compile_test.go @@ -12,7 +12,6 @@ import ( "maps" "reflect" "slices" - "sort" "strconv" "strings" "testing" @@ -746,9 +745,7 @@ func TestRuleTreeWithDotsInHeads(t *testing.T) { tree := c.RuleTree tree.DepthFirst(func(n *TreeNode) bool { t.Log(n) - if !sort.SliceIsSorted(n.Sorted, func(i, j int) bool { - return n.Sorted[i].Compare(n.Sorted[j]) < 0 - }) { + if !slices.IsSortedFunc(n.Sorted, Value.Compare) { t.Errorf("expected sorted to be sorted: %v", n.Sorted) } return false @@ -1510,19 +1507,13 @@ func TestCompilerErrorLimit(t *testing.T) { c.Compile(modules) errs := c.Errors - exp := []string{ + exp := util.Sorted([]string{ "4:23: rego_unsafe_var_error: var x is unsafe", "4:23: rego_unsafe_var_error: var z is unsafe", "rego_compile_error: error limit reached", - } - - result := make([]string, 0, len(errs)) - for _, err := range errs { - result = append(result, err.Error()) - } + }) + result := util.Sorted(util.Map(errs, (*Error).Error)) - sort.Strings(exp) - sort.Strings(result) if !slices.Equal(exp, result) { t.Errorf("Expected errors %v, got %v", exp, result) } @@ -1550,7 +1541,7 @@ i.j.k contains x7 if true return fmt.Sprintf("rego_unsafe_var_error: var %v is unsafe", v) } - expected := []string{ + expected := util.Sorted([]string{ makeErrMsg("x1"), makeErrMsg("x2"), makeErrMsg("x3"), @@ -1560,11 +1551,9 @@ i.j.k contains x7 if true makeErrMsg("x7"), makeErrMsg("eq"), makeErrMsg("else_var"), - } + }) result := compilerErrsToStringSlice(c.Errors) - sort.Strings(expected) - if len(result) != len(expected) { t.Fatalf("Expected %d:\n%v\nBut got %d:\n%v", len(expected), strings.Join(expected, "\n"), len(result), strings.Join(result, "\n")) } @@ -1803,7 +1792,6 @@ func TestCompilerCheckSafetyBodyErrors(t *testing.T) { for _, tc := range tests { t.Run(tc.note, func(t *testing.T) { - // Build slice of expected error messages. expected := []string{} @@ -1812,7 +1800,7 @@ func TestCompilerCheckSafetyBodyErrors(t *testing.T) { return nil }) // cannot return error - sort.Strings(expected) + slices.Sort(expected) // Compile test module. opts := ParserOptions{ @@ -3155,8 +3143,8 @@ p := [data() | data := 1]`, result = append(result, compiler.Errors[i].Message) } - sort.Strings(tc.expectedErrors) - sort.Strings(result) + slices.Sort(tc.expectedErrors) + slices.Sort(result) if len(tc.expectedErrors) != len(result) { t.Fatalf("Expected %d errors but got %d:\n\n%v\n\nGot:\n\n%v", @@ -5884,7 +5872,7 @@ func TestRewriteLocalVarDeclarationErrors(t *testing.T) { compileStages(c, StageRewriteLocalVars) - expectedErrors := []string{ + expectedErrors := util.Sorted([]string{ "var r1 referenced above", "var r2 assigned above", "var foo referenced above", @@ -5902,18 +5890,14 @@ func TestRewriteLocalVarDeclarationErrors(t *testing.T) { "cannot assign to boolean", "cannot assign to string", "cannot assign to null", - } - - sort.Strings(expectedErrors) + }) result := make([]string, 0, len(c.Errors)) - for i := range c.Errors { result = append(result, c.Errors[i].Message) } - sort.Strings(result) - + slices.Sort(result) if len(expectedErrors) != len(result) { t.Fatalf("Expected %d errors but got %d:\n\n%v\n\nGot:\n\n%v", len(expectedErrors), len(result), strings.Join(expectedErrors, "\n"), strings.Join(result, "\n")) } @@ -10183,7 +10167,7 @@ dataref = true if { data }`, return fmt.Sprintf("rego_recursion_error: rule data.%s.%s is recursive: %v", pkg, rule, strings.Join(l, " -> ")) } - expected := []string{ + expected := util.Sorted([]string{ makeRuleErrMsg("rec", "s", "s", "t", "s"), makeRuleErrMsg("rec", "t", "t", "s", "t"), makeRuleErrMsg("rec", "a", "a", "b", "c", "e", "a"), @@ -10210,11 +10194,9 @@ dataref = true if { data }`, makeRuleErrMsg("f2", "p[x]", "p[x]", "foo", "bar", "p[x]"), makeRuleErrMsg("everymod", "everyp", "everyp", "everyp"), makeRuleErrMsg("everymod", "everyq", "everyq", "everyq"), - } + }) result := compilerErrsToStringSlice(c.Errors) - sort.Strings(expected) - if len(result) != len(expected) { t.Fatalf("Expected %d:\n%v\nBut got %d:\n%v", len(expected), strings.Join(expected, "\n"), len(result), strings.Join(result, "\n")) } @@ -11993,12 +11975,7 @@ func getCompilerWithParsedModules(mods map[string]string) *Compiler { func compileStages(c *Compiler, stageID StageID) { c.init() - c.sorted = make([]string, 0, len(c.Modules)) - for name := range c.Modules { - c.sorted = append(c.sorted, name) - } - sort.Strings(c.sorted) - + c.sorted = util.KeysSorted(c.Modules) c = c.SetErrorLimit(0) // Tests need to see all errors, not just the first few if stageID != "" { @@ -12096,8 +12073,7 @@ func compilerErrsToStringSlice(errors []*Error) []string { msg := strings.SplitN(e.Error(), ":", 3)[2] result = append(result, strings.TrimSpace(msg)) } - sort.Strings(result) - return result + return util.Sorted(result) } func runQueryCompilerTest(q string, popts ParserOptions, pkg string, imports []string, expected any) func(*testing.T) { diff --git a/v1/ast/index_debug.go b/v1/ast/index_debug.go index c707dea5b57..17d74072eae 100644 --- a/v1/ast/index_debug.go +++ b/v1/ast/index_debug.go @@ -6,7 +6,7 @@ package ast import ( "fmt" - "sort" + "slices" "strings" "github.com/open-policy-agent/opa/v1/util" @@ -67,8 +67,8 @@ func (node *trieNode) mermaidFormat(sb *strings.Builder, counter *int, nodeIDs m pairs = append(pairs, scalarPair{key, val}) return false }) - sort.Slice(pairs, func(a, b int) bool { - return pairs[a].key.Compare(pairs[b].key) < 0 + slices.SortFunc(pairs, func(a, b scalarPair) int { + return a.key.Compare(b.key) }) for _, pair := range pairs { var scalarLabel string @@ -199,9 +199,7 @@ func (node *trieNode) format(sb *strings.Builder, depth int) { nodes = append(nodes, val) return false }) - sort.Slice(scalars, func(a, b int) bool { - return scalars[a].Compare(scalars[b]) < 0 - }) + slices.SortFunc(scalars, Value.Compare) for i := range scalars { sb.WriteString(indent) sb.WriteString(" ") diff --git a/v1/ast/map_test.go b/v1/ast/map_test.go index 29a58bd2caf..4173463cef3 100644 --- a/v1/ast/map_test.go +++ b/v1/ast/map_test.go @@ -5,9 +5,10 @@ package ast import ( - "reflect" - "sort" + "slices" "testing" + + "github.com/open-policy-agent/opa/v1/util" ) func TestValueMapOverwrite(t *testing.T) { @@ -31,9 +32,7 @@ func TestValueMapIter(t *testing.T) { values = append(values, string(v.(String))) return false }) - sort.Strings(values) - expected := []string{"bar", "baz", "foo"} - if !reflect.DeepEqual(values, expected) { + if values = util.Sorted(values); !slices.Equal(values, []string{"bar", "baz", "foo"}) { t.Fatalf("Unexpected value from iteration: %v", values) } } diff --git a/v1/ast/parser.go b/v1/ast/parser.go index ff468f7bba7..97a137e9526 100644 --- a/v1/ast/parser.go +++ b/v1/ast/parser.go @@ -15,7 +15,6 @@ import ( "net/url" "regexp" "slices" - "sort" "strconv" "strings" "unicode/utf8" @@ -3842,10 +3841,8 @@ func (p *Parser) futureImport(imp *Import, allowedFutureKeywords map[string]toke return } keyword := string(kw) - _, ok = allowedFutureKeywords[keyword] - if !ok { - sort.Strings(kwds) // so the error message is stable - p.errorf(imp.Path.Location, "unexpected keyword, must be one of %v", kwds) + if _, ok = allowedFutureKeywords[keyword]; !ok { + p.errorf(imp.Path.Location, "unexpected keyword, must be one of %v", util.Sorted(kwds)) return } diff --git a/v1/ast/treenode_dump.go b/v1/ast/treenode_dump.go index e91a35d6c68..0b7c5a6e603 100644 --- a/v1/ast/treenode_dump.go +++ b/v1/ast/treenode_dump.go @@ -2,7 +2,6 @@ package ast import ( "fmt" - "sort" "strings" "github.com/open-policy-agent/opa/v1/util" @@ -35,13 +34,7 @@ func (n *TreeNode) dumpRecursive(sb *strings.Builder, prefix, childPrefix string return } - keys := make([]Value, 0, len(n.Children)) - for k := range n.Children { - keys = append(keys, k) - } - sort.Slice(keys, func(i, j int) bool { - return Compare(keys[i], keys[j]) < 0 - }) + keys := util.SortedFunc(util.Keys(n.Children), Value.Compare) for i, key := range keys { child := n.Children[key] diff --git a/v1/bundle/file.go b/v1/bundle/file.go index 6dea6eb410c..beb2e466451 100644 --- a/v1/bundle/file.go +++ b/v1/bundle/file.go @@ -3,13 +3,14 @@ package bundle import ( "archive/tar" "bytes" + "cmp" "compress/gzip" "fmt" "io" "io/fs" "os" "path/filepath" - "sort" + "slices" "strings" "sync" @@ -439,13 +440,13 @@ func (it *iterator) Next() (*storage.Update, error) { } f.path = p - f.raw = item.Value - it.files = append(it.files, f) } - sortFilePathAscend(it.files) + slices.SortFunc(it.files, func(a, b file) int { + return cmp.Compare(len(a.path), len(b.path)) + }) } // If done reading files then just return io.EOF @@ -482,12 +483,6 @@ func NewIterator(raw []Raw) storage.Iterator { return &it } -func sortFilePathAscend(files []file) { - sort.Slice(files, func(i, j int) bool { - return len(files[i].path) < len(files[j].path) - }) -} - func getdepth(path string, isDir bool) int { if isDir { cleanedPath := strings.Trim(filepath.ToSlash(path), "/") diff --git a/v1/bundle/store.go b/v1/bundle/store.go index a753896cb40..156ca16d404 100644 --- a/v1/bundle/store.go +++ b/v1/bundle/store.go @@ -12,7 +12,7 @@ import ( "fmt" "maps" "path/filepath" - "sort" + "slices" "strings" "sync" @@ -1163,11 +1163,11 @@ func hasRootsOverlap(ctx context.Context, store storage.Store, txn storage.Trans } // Sort the bundle roots list. - sort.Slice(entries, func(i, j int) bool { - if entries[i].canonical != entries[j].canonical { - return entries[i].canonical < entries[j].canonical + slices.SortFunc(entries, func(a, b rootEntry) int { + if c := strings.Compare(a.canonical, b.canonical); c != 0 { + return c } - return entries[i].bundle < entries[j].bundle + return strings.Compare(a.bundle, b.bundle) }) collidingBundles := map[string]bool{} @@ -1226,8 +1226,7 @@ func hasRootsOverlap(ctx context.Context, store storage.Store, txn storage.Trans // is allowed to declare overlapping roots in its own manifest. if sawCrossBundleConflict { collidingBundles[entries[d].bundle] = true - paths := []string{groupDisplay, entries[d].displayRoot()} - sort.Strings(paths) + paths := util.Sorted([]string{groupDisplay, entries[d].displayRoot()}) conflictSet[fmt.Sprintf("%s overlaps %s", paths[0], paths[1])] = true } } diff --git a/v1/compile/compile.go b/v1/compile/compile.go index a2ceff27c37..5c7b9df99fb 100644 --- a/v1/compile/compile.go +++ b/v1/compile/compile.go @@ -16,7 +16,6 @@ import ( "path/filepath" "regexp" "slices" - "sort" "strings" "google.golang.org/protobuf/proto" @@ -552,15 +551,9 @@ func (c *Compiler) initBundle(usePath bool) error { } if c.asBundle { - var names []string - - for k := range load.Bundles { - names = append(names, k) - } - - sort.Strings(names) - var bundles []*bundle.Bundle + names := util.KeysSorted(load.Bundles) + bundles := make([]*bundle.Bundle, 0, len(names)) for _, k := range names { bundles = append(bundles, load.Bundles[k]) } @@ -586,14 +579,7 @@ func (c *Compiler) initBundle(usePath bool) error { result.Manifest.Init() result.Data = load.Files.Documents - modules := make([]string, 0, len(load.Files.Modules)) - for k := range load.Files.Modules { - modules = append(modules, k) - } - - sort.Strings(modules) - - for _, module := range modules { + for _, module := range util.KeysSorted(load.Files.Modules) { path := filepath.ToSlash(load.Files.Modules[module].Name) result.Modules = append(result.Modules, bundle.ModuleFile{ URL: path, @@ -1103,8 +1089,8 @@ func (o *optimizer) Do(ctx context.Context) error { o.bundle.Modules = o.merge(o.bundle.Modules, modules) } - sort.Slice(o.bundle.Modules, func(i, j int) bool { - return o.bundle.Modules[i].URL < o.bundle.Modules[j].URL + slices.SortFunc(o.bundle.Modules, func(a, b bundle.ModuleFile) int { + return strings.Compare(a.URL, b.URL) }) // NOTE(tsandall): prune out rules and data that are not referenced in the bundle @@ -1120,7 +1106,6 @@ func (o *optimizer) Bundle() *bundle.Bundle { } func (o *optimizer) findRequiredDocuments(ref *ast.Term) []string { - keep := map[string]*ast.Location{} deps := map[*ast.Rule]struct{}{} @@ -1140,13 +1125,7 @@ func (o *optimizer) findRequiredDocuments(ref *ast.Term) []string { }) } - result := make([]string, 0, len(keep)) - - for k := range keep { - result = append(result, k) - } - - sort.Strings(result) + result := util.KeysSorted(keep) for _, k := range result { o.debug.Printf("%s: disables inlining of %v", keep[k], k) @@ -1354,13 +1333,7 @@ type orderedStringSet []string func (ss orderedStringSet) Append(s ...string) orderedStringSet { for _, x := range s { - var found bool - for _, other := range ss { - if x == other { - found = true - } - } - if !found { + if !slices.Contains(ss, x) { ss = append(ss, x) } } @@ -1418,8 +1391,5 @@ func (rs *refSet) Sorted() []*ast.Term { for i := range rs.s { terms[i] = ast.NewTerm(rs.s[i]) } - sort.Slice(terms, func(i, j int) bool { - return terms[i].Value.Compare(terms[j].Value) < 0 - }) - return terms + return util.SortedFunc(terms, ast.TermValueCompare) } diff --git a/v1/dependencies/deps_test.go b/v1/dependencies/deps_test.go index f183e448bcb..1e3bad20e7c 100644 --- a/v1/dependencies/deps_test.go +++ b/v1/dependencies/deps_test.go @@ -6,7 +6,6 @@ package dependencies import ( "slices" - "sort" "strconv" "testing" @@ -349,14 +348,11 @@ func TestDependencies(t *testing.T) { t.Fatalf("Failed to compile policy: %v", compiler.Errors) } - var exp []ast.Ref + exp := make([]ast.Ref, 0, len(test.min)) for _, e := range test.min { - r := ast.MustParseRef("data." + e) - exp = append(exp, r) + exp = append(exp, ast.MustParseRef("data."+e)) } - sort.Slice(exp, func(i, j int) bool { - return exp[i].Compare(exp[j]) < 0 - }) + slices.SortFunc(exp, ast.RefCompare) mod := compiler.Modules["test"] minOfAll, full := runDeps(t, mod) @@ -378,9 +374,7 @@ func TestDependencies(t *testing.T) { r := ast.MustParseRef("data." + full) exp = append(exp, r) } - sort.Slice(exp, func(i, j int) bool { - return exp[i].Compare(exp[j]) < 0 - }) + slices.SortFunc(exp, ast.RefCompare) assertRefSliceEq(t, exp, full) assertRefSliceEq(t, exp, fullRules) diff --git a/v1/format/format.go b/v1/format/format.go index e2f74591aa4..3e3e09a499e 100644 --- a/v1/format/format.go +++ b/v1/format/format.go @@ -10,7 +10,6 @@ import ( "errors" "fmt" "slices" - "sort" "strings" "unicode" @@ -446,20 +445,20 @@ func (w *writer) writeModule(module *ast.Module) error { }) visitor.Walk(module) - sort.Slice(comments, func(i, j int) bool { - l, err := locLess(comments[i], comments[j]) + slices.SortFunc(comments, func(a, b *ast.Comment) int { + al, bl, err := getLocs(a, b) if err != nil { w.errs = append(w.errs, ast.NewError(ast.FormatErr, &ast.Location{}, "%s", err.Error())) } - return l + return locCmp(al, bl) }) - sort.Slice(others, func(i, j int) bool { - l, err := locLess(others[i], others[j]) + slices.SortFunc(others, func(a, b any) int { + al, bl, err := getLocs(a, b) if err != nil { w.errs = append(w.errs, ast.NewError(ast.FormatErr, &ast.Location{}, "%s", err.Error())) } - return l + return locCmp(al, bl) }) comments = trimTrailingWhitespaceInComments(comments) @@ -536,7 +535,7 @@ func (w *writer) writeComments(comments []*ast.Comment) error { var inMetadataBlock bool for i := range comments { if i > 0 { - l, err := locCmp(comments[i], comments[i-1]) + l, err := locCmpOrError(comments[i], comments[i-1]) if err != nil { return err } @@ -2182,7 +2181,7 @@ func (w *writer) groupIterable(elements []any, last *ast.Location) ([][]any, err } slices.SortFunc(elements, func(i, j any) int { - l, err := locCmp(i, j) + l, err := locCmpOrError(i, j) if err != nil { w.errs = append(w.errs, ast.NewError(ast.FormatErr, &ast.Location{}, "%s", err.Error())) } @@ -2328,34 +2327,27 @@ loop: return rules, others[i:] } -func locLess(a, b any) (bool, error) { - c, err := locCmp(a, b) - return c < 0, err -} - -func locCmp(a, b any) (int, error) { - al, err := getLoc(a) - if err != nil { - return 0, err - } - bl, err := getLoc(b) +func locCmpOrError(a, b any) (int, error) { + al, bl, err := getLocs(a, b) if err != nil { return 0, err } + return locCmp(al, bl), nil +} + +func locCmp(a, b *ast.Location) int { switch { - case al == nil && bl == nil: - return 0, nil - case al == nil: - return -1, nil - case bl == nil: - return 1, nil + case a == b: + return 0 + case a == nil: + return -1 + case b == nil: + return 1 } - - if cmp := al.Row - bl.Row; cmp != 0 { - return cmp, nil - + if cmp := a.Row - b.Row; cmp != 0 { + return cmp } - return al.Col - bl.Col, nil + return a.Col - b.Col } func getLoc(x any) (*ast.Location, error) { @@ -2371,6 +2363,12 @@ func getLoc(x any) (*ast.Location, error) { } } +func getLocs(a, b any) (*ast.Location, *ast.Location, error) { + al, err1 := getLoc(a) + bl, err2 := getLoc(b) + return al, bl, errors.Join(err1, err2) +} + var negativeRow = &ast.Location{Row: -1} func closingLoc(skipOpen, skipClose, openChar, closeChar byte, loc *ast.Location) *ast.Location { diff --git a/v1/loader/loader_test.go b/v1/loader/loader_test.go index a572caebb92..79188526afa 100644 --- a/v1/loader/loader_test.go +++ b/v1/loader/loader_test.go @@ -16,7 +16,6 @@ import ( "path/filepath" "reflect" "slices" - "sort" "strings" "testing" @@ -1072,8 +1071,7 @@ func TestLoadRooted(t *testing.T) { } test.WithTempFS(files, func(rootDir string) { - paths := mustListPaths(rootDir, false)[1:] - sort.Strings(paths) + paths := util.Sorted(mustListPaths(rootDir, false)[1:]) paths[0] = "one.two:" + paths[0] paths[1] = "three:" + paths[1] paths[2] = "four:" + paths[2] @@ -1190,8 +1188,7 @@ func TestGlobExcludeName(t *testing.T) { } test.WithTempFS(files, func(rootDir string) { - paths := mustListPaths(rootDir, false)[1:] - sort.Strings(paths) + paths := util.Sorted(mustListPaths(rootDir, false)[1:]) result, err := NewFileLoader().Filtered(paths, GlobExcludeName(".*", 1)) if err != nil { t.Fatal(err) @@ -1220,9 +1217,7 @@ func TestLoadErrors(t *testing.T) { "/bad_doc.json": "[1,2,3]", } test.WithTempFS(files, func(rootDir string) { - paths := mustListPaths(rootDir, false)[1:] - sort.Strings(paths) - _, err := NewFileLoader().All(paths) + _, err := NewFileLoader().All(util.Sorted(mustListPaths(rootDir, false)[1:])) if err == nil { t.Fatalf("Expected failure") } @@ -1251,10 +1246,7 @@ func TestLoadFileURL(t *testing.T) { "c.json": `3`, // this will loas as rooted file } test.WithTempFS(files, func(rootDir string) { - - paths := mustListPaths(rootDir, false)[1:] - sort.Strings(paths) - + paths := util.Sorted(mustListPaths(rootDir, false)[1:]) for i := range paths { paths[i] = "file://" + paths[i] } @@ -1351,9 +1343,7 @@ func TestLoadRegos(t *testing.T) { } test.WithTempFS(files, func(rootDir string) { - paths := mustListPaths(rootDir, false)[1:] - sort.Strings(paths) - result, err := AllRegos(paths) + result, err := AllRegos(util.Sorted(mustListPaths(rootDir, false)[1:])) if err != nil { t.Fatal(err) } diff --git a/v1/plugins/bundle/plugin_test.go b/v1/plugins/bundle/plugin_test.go index c2474dfedf4..0cb4ab98fb4 100644 --- a/v1/plugins/bundle/plugin_test.go +++ b/v1/plugins/bundle/plugin_test.go @@ -18,7 +18,6 @@ import ( "path/filepath" "reflect" "slices" - "sort" "strings" "testing" "time" @@ -4464,8 +4463,8 @@ func TestReconfigurePlugin_OneShot_BundleDeactivation(t *testing.T) { expIDs := []string{"test-bundle/bundle/id1"} - sort.Strings(ids) - sort.Strings(expIDs) + slices.Sort(ids) + slices.Sort(expIDs) if !slices.Equal(ids, expIDs) { t.Fatalf("expected ids %v but got %v", expIDs, ids) @@ -4490,7 +4489,7 @@ func TestReconfigurePlugin_OneShot_BundleDeactivation(t *testing.T) { expIDs = []string{} - sort.Strings(ids) + slices.Sort(ids) if !slices.Equal(ids, expIDs) { t.Fatalf("expected ids %v but got %v", expIDs, ids) @@ -4630,8 +4629,8 @@ func TestReconfigurePlugin_ManagerInit_BundleDeactivation(t *testing.T) { expIDs := []string{"test-bundle/bundle/id1"} - sort.Strings(ids) - sort.Strings(expIDs) + slices.Sort(ids) + slices.Sort(expIDs) if !slices.Equal(ids, expIDs) { t.Fatalf("expected ids %v but got %v", expIDs, ids) @@ -4656,7 +4655,7 @@ func TestReconfigurePlugin_ManagerInit_BundleDeactivation(t *testing.T) { expIDs = []string{} - sort.Strings(ids) + slices.Sort(ids) if !slices.Equal(ids, expIDs) { t.Fatalf("expected ids %v but got %v", expIDs, ids) @@ -7762,8 +7761,8 @@ func validateStoreState(ctx context.Context, t *testing.T, store storage.Store, return err } - sort.Strings(ids) - sort.Strings(expIDs) + slices.Sort(ids) + slices.Sort(expIDs) if !slices.Equal(ids, expIDs) { return fmt.Errorf("expected ids %v but got %v", expIDs, ids) diff --git a/v1/profiler/profiler.go b/v1/profiler/profiler.go index b7072df67d8..11d3b5adc45 100644 --- a/v1/profiler/profiler.go +++ b/v1/profiler/profiler.go @@ -6,13 +6,13 @@ package profiler import ( - "slices" - "sort" + "cmp" "time" "github.com/open-policy-agent/opa/v1/ast" "github.com/open-policy-agent/opa/v1/metrics" "github.com/open-policy-agent/opa/v1/topdown" + "github.com/open-policy-agent/opa/v1/util" ) var unknownLocation = ast.NewLocation([]byte("???"), "", 0, 0) @@ -57,7 +57,7 @@ func (*Profiler) Config() topdown.TraceConfig { func (p *Profiler) ReportByFile() Report { p.processLastExpr() - report := Report{Files: map[string]*FileReport{}} + report := Report{Files: make(map[string]*FileReport, len(p.hits))} for file, hits := range p.hits { stats := make([]ExprStats, 0, len(hits)) @@ -68,13 +68,12 @@ func (p *Profiler) ReportByFile() Report { stats = append(stats, stat) } - sortStatsByRow(stats) fr, ok := report.Files[file] if !ok { fr = &FileReport{} report.Files[file] = fr } - fr.Result = stats + fr.Result = util.SortedFunc(stats, cmpLineAsc) } return report @@ -85,8 +84,12 @@ func (p *Profiler) ReportByFile() Report { func (p *Profiler) ReportTopNResults(numResults int, criteria []string) []ExprStats { p.processLastExpr() - stats := []ExprStats{} + n := 0 + for _, hits := range p.hits { + n += len(hits) + } + stats := make(exprStatsSlice, 0, n) for file, hits := range p.hits { for row, stat := range hits { if entry, ok := p.hitsByExprIndex[file][row]; ok { @@ -96,50 +99,7 @@ func (p *Profiler) ReportTopNResults(numResults int, criteria []string) []ExprSt } } - // allowed criteria for sorting results - allowedCriteria := map[string]lessFunc{} - allowedCriteria["total_time_ns"] = func(stat1, stat2 *ExprStats) bool { - return stat1.ExprTimeNs > stat2.ExprTimeNs - } - allowedCriteria["num_eval"] = func(stat1, stat2 *ExprStats) bool { - return stat1.NumEval > stat2.NumEval - } - allowedCriteria["num_redo"] = func(stat1, stat2 *ExprStats) bool { - return stat1.NumRedo > stat2.NumRedo - } - allowedCriteria["num_gen_expr"] = func(stat1, stat2 *ExprStats) bool { - return stat1.NumGenExpr > stat2.NumGenExpr - } - allowedCriteria["file"] = func(stat1, stat2 *ExprStats) bool { - return stat1.Location.File > stat2.Location.File - } - allowedCriteria["line"] = func(stat1, stat2 *ExprStats) bool { - return stat1.Location.Row > stat2.Location.Row - } - - sortFuncs := []lessFunc{} - - for _, cr := range criteria { - if fn, ok := allowedCriteria[cr]; ok { - sortFuncs = append(sortFuncs, fn) - } - } - - // if no criteria return all the stats - if len(sortFuncs) == 0 { - return stats - } - - orderedBy(sortFuncs).Sort(stats) - - // if desired number of results to be returned is less than or - // equal to 0 or exceed total available results, - // return all the stats - if numResults <= 0 || numResults > len(stats) { - return stats - } - return stats[:numResults] - + return stats.orderedBy(criteria...).limit(numResults) } // Trace updates the profiler state. @@ -222,14 +182,12 @@ func (p *Profiler) processLastExpr() { func (p *Profiler) calculateHitsByExprIndex() { file := p.prevExpr.location.File + hitsUnique, ok := p.hitsByExprIndex[file] if !ok { - hitsUnique = map[int]map[int]ExprStats{ - p.prevExpr.location.Row: { - p.prevExpr.index: getProfilerStats(p.prevExpr, p.activeTimer), - }, + p.hitsByExprIndex[file] = map[int]map[int]ExprStats{ + p.prevExpr.location.Row: {p.prevExpr.index: getProfilerStats(p.prevExpr, p.activeTimer)}, } - p.hitsByExprIndex[file] = hitsUnique } else { row := p.prevExpr.location.Row idx := p.prevExpr.index @@ -258,9 +216,10 @@ func (p *Profiler) calculateHitsByExprIndex() { } func getProfilerStats(expr exprInfo, timer time.Time) ExprStats { - profilerStats := ExprStats{} - profilerStats.ExprTimeNs = time.Since(timer).Nanoseconds() - profilerStats.Location = expr.location + profilerStats := ExprStats{ + ExprTimeNs: time.Since(timer).Nanoseconds(), + Location: expr.location, + } switch expr.op { case topdown.EvalOp: @@ -280,6 +239,8 @@ type ExprStats struct { Location *ast.Location `json:"location"` } +type exprStatsSlice []ExprStats + // ExprStatsAggregated represents the result of profiling an expression // by aggregating `n` profiles. type ExprStatsAggregated struct { @@ -294,41 +255,33 @@ func aggregate(stats ...ExprStats) ExprStatsAggregated { if len(stats) == 0 { return ExprStatsAggregated{} } - res := ExprStatsAggregated{ - NumEval: stats[0].NumEval, - NumRedo: stats[0].NumRedo, - NumGenExpr: stats[0].NumGenExpr, - Location: stats[0].Location, - } timeNs := make([]int64, 0, len(stats)) for _, s := range stats { timeNs = append(timeNs, s.ExprTimeNs) } - res.ExprTimeNsStats = metrics.Statistics(timeNs...) - return res + return ExprStatsAggregated{ + NumEval: stats[0].NumEval, + NumRedo: stats[0].NumRedo, + NumGenExpr: stats[0].NumGenExpr, + Location: stats[0].Location, + ExprTimeNsStats: metrics.Statistics(timeNs...), + } } -func AggregateProfiles(profiles ...[]ExprStats) []ExprStatsAggregated { - if len(profiles) == 0 { - return []ExprStatsAggregated{} - } - res := make([]ExprStatsAggregated, len(profiles[0])) - for j := range len(profiles[0]) { - var s []ExprStats - for _, p := range profiles { - s = append(s, p[j]) +func AggregateProfiles(profiles ...[]ExprStats) (res []ExprStatsAggregated) { + if len(profiles) > 0 { + res = make([]ExprStatsAggregated, len(profiles[0])) + for j := range profiles[0] { + s := make(exprStatsSlice, 0, len(profiles)) + for _, p := range profiles { + s = append(s, p[j]) + } + res[j] = aggregate(s...) } - res[j] = aggregate(s...) } return res } -func sortStatsByRow(ps []ExprStats) { - slices.SortFunc(ps, func(stat1, stat2 ExprStats) int { - return stat1.Location.Row - stat2.Location.Row - }) -} - // Report represents the profiler report for a set of files. type Report struct { Files map[string]*FileReport `json:"files"` @@ -339,59 +292,68 @@ type FileReport struct { Result []ExprStats `json:"result"` } -// Helper interfaces and methods for sorting a slice of ExprStats structs -// based on multiple fields. +func (e exprStatsSlice) orderedBy(criteria ...string) exprStatsSlice { + if len(criteria) == 0 { + return e + } -type lessFunc func(p1, p2 *ExprStats) bool + allComparers := map[string]func(p1, p2 ExprStats) int{ + "total_time_ns": cmpTotalTimeNs, + "num_eval": cmpNumEval, + "num_redo": cmpNumRedo, + "num_gen_expr": cmpNumGenExpr, + "file": cmpFile, + "line": cmpLineDsc, + } -// multiSorter implements the Sort interface, sorting the changes within. -type multiSorter struct { - stats []ExprStats - less []lessFunc -} + criteriaComparers := make([]func(ExprStats, ExprStats) int, 0, len(criteria)) + for _, c := range criteria { + if fn, ok := allComparers[c]; ok { + criteriaComparers = append(criteriaComparers, fn) + } + } -// Sort sorts the argument slice according to the less functions passed to OrderedBy. -func (ms *multiSorter) Sort(stats []ExprStats) { - ms.stats = stats - sort.Sort(ms) + return util.SortedFunc(e, func(a, b ExprStats) int { + for _, comparer := range criteriaComparers { + if res := comparer(a, b); res != 0 { + return res + } + } + return 0 + }) } -// orderedBy returns a Sorter that sorts using the less functions, in order. -func orderedBy(less []lessFunc) *multiSorter { - return &multiSorter{ - less: less, +func (e exprStatsSlice) limit(n int) exprStatsSlice { + if n <= 0 { + return e } + return e[:min(n, len(e))] } -// Len is part of sort.Interface. -func (ms *multiSorter) Len() int { - return len(ms.stats) +func cmpTotalTimeNs(stat1, stat2 ExprStats) int { + return int(stat2.ExprTimeNs - stat1.ExprTimeNs) } -// Swap is part of sort.Interface. -func (ms *multiSorter) Swap(i, j int) { - ms.stats[i], ms.stats[j] = ms.stats[j], ms.stats[i] +func cmpNumEval(stat1, stat2 ExprStats) int { + return stat2.NumEval - stat1.NumEval } -// Less is part of sort.Interface. It is implemented by looping along the -// less functions until it finds a comparison that discriminates between -// the two items. -func (ms *multiSorter) Less(i, j int) bool { - p, q := &ms.stats[i], &ms.stats[j] - // Try all but the last comparison. - var k int - // changing this here changes the semantics, likely because - // k outlives the range.. seems like a bug in the intrange linter - //nolint:intrange - for k = 0; k < len(ms.less)-1; k++ { - less := ms.less[k] - switch { - case less(p, q): - return true - case less(q, p): - return false - } - // p == q; try the next comparison. - } - return ms.less[k](p, q) +func cmpNumRedo(stat1, stat2 ExprStats) int { + return stat2.NumRedo - stat1.NumRedo +} + +func cmpNumGenExpr(stat1, stat2 ExprStats) int { + return stat2.NumGenExpr - stat1.NumGenExpr +} + +func cmpFile(stat1, stat2 ExprStats) int { + return cmp.Compare(stat2.Location.File, stat1.Location.File) +} + +func cmpLineDsc(stat1, stat2 ExprStats) int { + return cmp.Compare(stat2.Location.Row, stat1.Location.Row) +} + +func cmpLineAsc(stat1, stat2 ExprStats) int { + return cmp.Compare(stat1.Location.Row, stat2.Location.Row) } diff --git a/v1/rego/example_test.go b/v1/rego/example_test.go index 76ab995c08e..c42b4d7d1c0 100644 --- a/v1/rego/example_test.go +++ b/v1/rego/example_test.go @@ -11,7 +11,7 @@ import ( "encoding/json" "fmt" "os" - "sort" + "slices" "strings" "github.com/open-policy-agent/opa/v1/logging" @@ -814,7 +814,7 @@ func makeStable(bodies []ast.Body) { return false // go on }) } - sort.Slice(bodies, func(i, j int) bool { return bodies[i].Compare(bodies[j]) < 0 }) + slices.SortFunc(bodies, ast.Body.Compare) } func ExampleRego_PrepareForPartial() { diff --git a/v1/sdk/test/test.go b/v1/sdk/test/test.go index 1db9dfac8c1..67173b87cf3 100644 --- a/v1/sdk/test/test.go +++ b/v1/sdk/test/test.go @@ -12,7 +12,7 @@ import ( "net/http/httptest" "os" "path/filepath" - "sort" + "slices" "strconv" "strings" "time" @@ -145,8 +145,8 @@ func (s *Server) buildBundles(ref string, policies map[string]string) error { Parsed: module, }) } - sort.Slice(modules, func(i, j int) bool { - return modules[i].URL < modules[j].URL + slices.SortFunc(modules, func(a, b bundle.ModuleFile) int { + return strings.Compare(a.URL, b.URL) }) // Compile the bundle out into a buffer @@ -450,8 +450,8 @@ func (s *Server) handleBundles(w http.ResponseWriter, r *http.Request) { return } } - sort.Slice(modules, func(i, j int) bool { - return modules[i].URL < modules[j].URL + slices.SortFunc(modules, func(a, b bundle.ModuleFile) int { + return strings.Compare(a.URL, b.URL) }) // Compile the bundle out into a buffer diff --git a/v1/test/cases/cases.go b/v1/test/cases/cases.go index 599fd74687e..f1d142f7ae4 100644 --- a/v1/test/cases/cases.go +++ b/v1/test/cases/cases.go @@ -9,7 +9,8 @@ import ( "fmt" "os" "path/filepath" - "sort" + "slices" + "strings" "github.com/open-policy-agent/opa/v1/util" ) @@ -25,10 +26,8 @@ type Set struct { // Sorted returns a sorted copy of s. func (s Set) Sorted() Set { - cpy := make([]TestCase, len(s.Cases)) - copy(cpy, s.Cases) - sort.Slice(cpy, func(i, j int) bool { - return cpy[i].Note < cpy[j].Note + cpy := util.SortedFunc(slices.Clone(s.Cases), func(a, b TestCase) int { + return strings.Compare(a.Note, b.Note) }) return Set{Cases: cpy} } diff --git a/v1/tester/reporter.go b/v1/tester/reporter.go index 96e4907ba4d..436f3f28286 100644 --- a/v1/tester/reporter.go +++ b/v1/tester/reporter.go @@ -11,7 +11,6 @@ import ( "fmt" "io" "slices" - "sort" "strings" "github.com/open-policy-agent/opa/cmd/formats" @@ -276,11 +275,7 @@ func (r JSONReporter) Report(ch chan *Result) error { switch r.Sort { case formats.SortDuration: slices.SortFunc(report, func(i, j *Result) int { - return cmp.Compare(i.Duration, j.Duration) - }) - - sort.Slice(report, func(i, j int) bool { - return report[i].Duration > report[j].Duration + return cmp.Compare(j.Duration, i.Duration) }) } diff --git a/v1/topdown/aggregates.go b/v1/topdown/aggregates.go index 65fda29bcee..d534539b3e5 100644 --- a/v1/topdown/aggregates.go +++ b/v1/topdown/aggregates.go @@ -7,6 +7,7 @@ package topdown import ( "math" "math/big" + "slices" "github.com/open-policy-agent/opa/v1/ast" "github.com/open-policy-agent/opa/v1/topdown/builtins" @@ -206,7 +207,7 @@ func builtinMax(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) err } max := ast.InternedNullTerm.Value a.Foreach(func(x *ast.Term) { - if ast.Compare(max, x.Value) <= 0 { + if max.Compare(x.Value) <= 0 { max = x.Value } }) @@ -215,16 +216,7 @@ func builtinMax(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) err if a.Len() == 0 { return nil } - max, err := a.Reduce(ast.InternedNullTerm, func(max *ast.Term, elem *ast.Term) (*ast.Term, error) { - if ast.Compare(max, elem) <= 0 { - return elem, nil - } - return max, nil - }) - if err != nil { - return err - } - return iter(max) + return iter(slices.MaxFunc(a.Slice(), ast.TermValueCompare)) } return builtins.NewOperandTypeErr(1, operands[0].Value, "set", "array") @@ -238,7 +230,7 @@ func builtinMin(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) err } min := a.Elem(0).Value a.Foreach(func(x *ast.Term) { - if ast.Compare(min, x.Value) >= 0 { + if min.Compare(x.Value) >= 0 { min = x.Value } }) @@ -247,23 +239,7 @@ func builtinMin(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) err if a.Len() == 0 { return nil } - min, err := a.Reduce(ast.InternedNullTerm, func(min *ast.Term, elem *ast.Term) (*ast.Term, error) { - // The null term is considered to be less than any other term, - // so in order for min of a set to make sense, we need to check - // for it. - if min.Value.Compare(ast.InternedNullValue) == 0 { - return elem, nil - } - - if ast.Compare(min, elem) >= 0 { - return elem, nil - } - return min, nil - }) - if err != nil { - return err - } - return iter(min) + return iter(slices.MinFunc(a.Slice(), ast.TermValueCompare)) } return builtins.NewOperandTypeErr(1, operands[0].Value, "set", "array") diff --git a/v1/topdown/cidr.go b/v1/topdown/cidr.go index e6e2bb3ae77..3f419a2931c 100644 --- a/v1/topdown/cidr.go +++ b/v1/topdown/cidr.go @@ -7,7 +7,6 @@ import ( "math/big" "net" "slices" - "sort" cidrMerge "github.com/open-policy-agent/opa/internal/cidr/merge" "github.com/open-policy-agent/opa/v1/ast" @@ -235,38 +234,15 @@ type cidrBlockRange struct { Network *net.IPNet } -type cidrBlockRanges []*cidrBlockRange - -// Implement Sort interface -func (c cidrBlockRanges) Len() int { - return len(c) -} - -func (c cidrBlockRanges) Swap(i, j int) { - c[i], c[j] = c[j], c[i] -} - -func (c cidrBlockRanges) Less(i, j int) bool { - // Compare last IP. - cmp := bytes.Compare(*c[i].Last, *c[j].Last) - if cmp < 0 { - return true - } else if cmp > 0 { - return false - } - - // Then compare first IP. - cmp = bytes.Compare(*c[i].First, *c[j].First) - if cmp < 0 { - return true - } else if cmp > 0 { - return false +func (c *cidrBlockRange) Compare(other *cidrBlockRange) int { + if cmp := bytes.Compare(*c.Last, *other.Last); cmp != 0 { // Compare last IP. + return cmp } - - // Ranges are Equal. - return false + return bytes.Compare(*c.First, *other.First) // Then compare first IP. } +type cidrBlockRanges []*cidrBlockRange + // builtinNetCIDRMerge merges the provided list of IP addresses and subnets into the smallest possible list of CIDRs. // It merges adjacent subnets where possible, those contained within others and also removes any duplicates. // Original Algorithm: https://github.com/netaddr/netaddr. @@ -283,16 +259,12 @@ func builtinNetCIDRMerge(_ BuiltinContext, operands []*ast.Term, iter func(*ast. networks = append(networks, network) } case ast.Set: - err := v.Iter(func(x *ast.Term) error { - network, err := generateIPNet(x) + for _, term := range v.Slice() { + network, err := generateIPNet(term) if err != nil { return err } networks = append(networks, network) - return nil - }) - if err != nil { - return err } default: return errors.New("operand must be an array") @@ -367,7 +339,7 @@ func generateIPNet(term *ast.Term) (*net.IPNet, error) { } func mergeCIDRs(ranges cidrBlockRanges) cidrBlockRanges { - sort.Sort(ranges) + slices.SortFunc(ranges, (*cidrBlockRange).Compare) // Merge adjacent CIDRs if possible. for i := len(ranges) - 1; i > 0; i-- { diff --git a/v1/topdown/copypropagation/copypropagation.go b/v1/topdown/copypropagation/copypropagation.go index 799a7161362..c52bd937cf9 100644 --- a/v1/topdown/copypropagation/copypropagation.go +++ b/v1/topdown/copypropagation/copypropagation.go @@ -6,7 +6,6 @@ package copypropagation import ( "fmt" - "sort" "github.com/open-policy-agent/opa/v1/ast" "github.com/open-policy-agent/opa/v1/util" @@ -475,10 +474,9 @@ func sortbindings(bindings *ast.ValueMap) []*binding { sorted = append(sorted, &binding{k, v}) return false }) - sort.Slice(sorted, func(i, j int) bool { - return sorted[i].k.Compare(sorted[j].k) > 0 + return util.SortedFunc(sorted, func(a, b *binding) int { + return b.k.Compare(a.k) }) - return sorted } // makeDisjointSets builds the union-find structure for the query. The structure diff --git a/v1/topdown/exported_test.go b/v1/topdown/exported_test.go index 47c095eba33..ef33440bc69 100644 --- a/v1/topdown/exported_test.go +++ b/v1/topdown/exported_test.go @@ -7,7 +7,7 @@ package topdown import ( "fmt" "os" - "sort" + "slices" "strings" "testing" @@ -16,6 +16,7 @@ import ( inmem "github.com/open-policy-agent/opa/v1/storage/inmem/test" "github.com/open-policy-agent/opa/v1/test/cases" "github.com/open-policy-agent/opa/v1/topdown/builtins" + "github.com/open-policy-agent/opa/v1/util" ) func TestRego(t *testing.T) { @@ -236,7 +237,7 @@ func testAssertResultSet(t *testing.T, wantResult []map[string]any, rs QueryResu t.Fatal(err) } if sortBindings { - sort.Sort(resultSet(v.([]any))) + slices.SortFunc(v.([]any), util.Compare) } name := string(k) if !ignoreGeneratedVars || !strings.HasPrefix(name, "__localq") { diff --git a/v1/topdown/query.go b/v1/topdown/query.go index 971d62b33bc..562aace9abc 100644 --- a/v1/topdown/query.go +++ b/v1/topdown/query.go @@ -4,7 +4,7 @@ import ( "context" "crypto/rand" "io" - "sort" + "slices" "time" "github.com/open-policy-agent/opa/v1/ast" @@ -492,9 +492,7 @@ func (q *Query) PartialRun(ctx context.Context) (partials []ast.Body, support [] }) // cannot return error // Sort binding expressions so that results are deterministic. - sort.Slice(bindingExprs, func(i, j int) bool { - return bindingExprs[i].Compare(bindingExprs[j]) < 0 - }) + slices.SortFunc(bindingExprs, (*ast.Expr).Compare) for i := range bindingExprs { body.Append(bindingExprs[i]) @@ -541,10 +539,7 @@ func (q *Query) PartialRun(ctx context.Context) (partials []ast.Body, support [] if regoVersion := q.compiler.DefaultRegoVersion(); regoVersion != ast.RegoUndefined { ast.SetModuleRegoVersion(m, q.compiler.DefaultRegoVersion()) } - - sort.Slice(support[i].Rules, func(j, k int) bool { - return support[i].Rules[j].Compare(support[i].Rules[k]) < 0 - }) + slices.SortFunc(support[i].Rules, (*ast.Rule).Compare) } return partials, support, err diff --git a/v1/topdown/topdown_test.go b/v1/topdown/topdown_test.go index 0426f41102a..01bc5bd624d 100644 --- a/v1/topdown/topdown_test.go +++ b/v1/topdown/topdown_test.go @@ -14,7 +14,6 @@ import ( "reflect" "runtime" "slices" - "sort" "strconv" "strings" "sync" @@ -1559,8 +1558,8 @@ arr := [1, 2, 3, 4, 5] exits[ev.Message]++ } } - sort.Strings(notes) - sort.Strings(tc.notes) + slices.Sort(notes) + slices.Sort(tc.notes) if !slices.Equal(notes, tc.notes) { t.Errorf("unexpected note traces, expected %v, got %v", tc.notes, notes) } @@ -2223,9 +2222,9 @@ func assertTopDownWithPathAndContext(ctx context.Context, t *testing.T, compiler expected := util.MustUnmarshalJSON([]byte(e)) if requiresSort { - sort.Sort(resultSet(result.([]any))) + slices.SortFunc(result.([]any), util.Compare) if sl, ok := expected.([]any); ok { - sort.Sort(resultSet(sl)) + slices.SortFunc(sl, util.Compare) } } @@ -2317,9 +2316,9 @@ func runTopDownPartialTestCase(ctx context.Context, t *testing.T, compiler *ast. } if requiresSort { - sort.Sort(resultSet(result.([]any))) + slices.SortFunc(result.([]any), util.Compare) if sl, ok := expected.([]any); ok { - sort.Sort(resultSet(sl)) + slices.SortFunc(sl, util.Compare) } } @@ -2328,22 +2327,7 @@ func runTopDownPartialTestCase(ctx context.Context, t *testing.T, compiler *ast. } } -type resultSet []any - -func (rs resultSet) Less(i, j int) bool { - return util.Compare(rs[i], rs[j]) < 0 -} - -func (rs resultSet) Swap(i, j int) { - rs[i], rs[j] = rs[j], rs[i] -} - -func (rs resultSet) Len() int { - return len(rs) -} - func init() { - ast.RegisterBuiltin(&ast.Builtin{ Name: "test.sleep", Decl: types.NewFunction( @@ -2405,8 +2389,7 @@ func dump(note string, modules map[string]*ast.Module, data any, docpath []strin if len(e) > 0 { exp := util.MustUnmarshalJSON([]byte(e)) if requiresSort { - sl := exp.([]any) - sort.Sort(resultSet(sl)) + slices.SortFunc(exp.([]any), util.Compare) } rs = append(rs, map[string]any{"x": exp}) } diff --git a/v1/types/types.go b/v1/types/types.go index 474265073ca..c02fee4d407 100644 --- a/v1/types/types.go +++ b/v1/types/types.go @@ -11,7 +11,6 @@ import ( "errors" "fmt" "slices" - "sort" "strings" "github.com/open-policy-agent/opa/v1/util" @@ -144,11 +143,8 @@ func NewBoolean() Boolean { } // MarshalJSON returns the JSON encoding of t. -func (t Boolean) MarshalJSON() ([]byte, error) { - repr := map[string]any{ - "type": t.typeMarker(), - } - return json.Marshal(repr) +func (Boolean) MarshalJSON() ([]byte, error) { + return util.StringToByteSlice(`{"type":"boolean"}`), nil } func (t Boolean) String() string { @@ -164,10 +160,8 @@ func NewString() String { } // MarshalJSON returns the JSON encoding of t. -func (t String) MarshalJSON() ([]byte, error) { - return json.Marshal(map[string]any{ - "type": t.typeMarker(), - }) +func (String) MarshalJSON() ([]byte, error) { + return util.StringToByteSlice(`{"type":"string"}`), nil } func (String) String() string { @@ -183,10 +177,8 @@ func NewNumber() Number { } // MarshalJSON returns the JSON encoding of t. -func (t Number) MarshalJSON() ([]byte, error) { - return json.Marshal(map[string]any{ - "type": t.typeMarker(), - }) +func (Number) MarshalJSON() ([]byte, error) { + return util.StringToByteSlice(`{"type":"number"}`), nil } func (Number) String() string { @@ -296,8 +288,7 @@ func (t *Set) toMap() map[string]any { } func (t *Set) String() string { - prefix := typeSet - return prefix + "[" + Sprint(t.of) + "]" + return typeSet + "[" + Sprint(t.of) + "]" } // StaticProperty represents a static object property. @@ -356,11 +347,8 @@ type Object struct { // NewObject returns a new Object type. func NewObject(static []*StaticProperty, dynamic *DynamicProperty) *Object { - slices.SortFunc(static, func(a, b *StaticProperty) int { - return util.Compare(a.Key, b.Key) - }) return &Object{ - static: static, + static: util.SortedFunc(static, cmpSpKey), dynamic: dynamic, } } @@ -428,18 +416,12 @@ func (t *Object) toMap() map[string]any { // Select returns the type of the named property. func (t *Object) Select(name any) Type { - pos := sort.Search(len(t.static), func(x int) bool { - return util.Compare(t.static[x].Key, name) >= 0 - }) - - if pos < len(t.static) && util.Compare(t.static[pos].Key, name) == 0 { + if pos, found := slices.BinarySearchFunc(t.static, name, cmpSpKeyName); found { return t.static[pos].Value } - if t.dynamic != nil { - if Contains(t.dynamic.Key, TypeOf(name)) { - return t.dynamic.Value - } + if t.dynamic != nil && Contains(t.dynamic.Key, TypeOf(name)) { + return t.dynamic.Value } return nil @@ -553,8 +535,7 @@ type Any []Type func NewAny(of ...Type) Any { sl := make(Any, len(of)) copy(sl, of) - sort.Sort(typeSlice(sl)) - return sl + return util.SortedFunc(sl, Compare) } // Contains returns true if t is a superset of other. @@ -562,16 +543,8 @@ func (t Any) Contains(other Type) bool { if _, ok := other.(*Function); ok { return false } - // Note(philipc): We used to do this as a linear search. - // Since this is always sorted, we can use a binary search instead. - i := sort.Search(len(t), func(i int) bool { - return Compare(t[i], other) >= 0 - }) - if i < len(t) && Compare(t[i], other) == 0 { - // x is present at t[i] - return true - } - return len(t) == 0 + _, found := slices.BinarySearchFunc(t, other, Compare) + return found || len(t) == 0 } // MarshalJSON returns the JSON encoding of t. @@ -598,9 +571,8 @@ func (t Any) Merge(other Type) Any { return t } cpy := make(Any, len(t)+1) - idx := sort.Search(len(t), func(i int) bool { - return Compare(t[i], other) >= 0 - }) + idx, _ := slices.BinarySearchFunc(t, other, Compare) + copy(cpy, t[:idx]) cpy[idx] = other copy(cpy[idx+1:], t[idx:]) @@ -910,7 +882,7 @@ func Compare(a, b Type) int { return cmp } } - return typeSliceCompare(arrA.static, arrB.static) + return slices.CompareFunc(arrA.static, arrB.static, Compare) case *Object: objA := a.(*Object) objB := b.(*Object) @@ -964,7 +936,7 @@ func Compare(a, b Type) int { } return Compare(setA.of, setB.of) case Any: - return typeSliceCompare(typeSlice(a.(Any)), typeSlice(b.(Any))) + return slices.CompareFunc([]Type(a.(Any)), []Type(b.(Any)), Compare) case *Function: fA := a.(*Function) fB := b.(*Function) @@ -1210,36 +1182,11 @@ func TypeOf(x any) Type { } return NewObject(static, nil) case []any: - static := make([]Type, len(x)) - for i := range x { - static[i] = TypeOf(x[i]) - } - return NewArray(static, nil) + return NewArray(util.Map(x, TypeOf), nil) } panic("unreachable") } -type typeSlice []Type - -func (s typeSlice) Less(i, j int) bool { return Compare(s[i], s[j]) < 0 } -func (s typeSlice) Swap(i, j int) { s[i], s[j] = s[j], s[i] } -func (s typeSlice) Len() int { return len(s) } - -func typeSliceCompare(a, b []Type) int { - minLen := min(len(b), len(a)) - for i := range minLen { - if cmp := Compare(a[i], b[i]); cmp != 0 { - return cmp - } - } - if len(a) < len(b) { - return -1 - } else if len(b) < len(a) { - return 1 - } - return 0 -} - func typeOrder(x Type) int { switch unwrapRecursive(unwrap(x)).(type) { case Null: @@ -1265,3 +1212,11 @@ func typeOrder(x Type) int { } panic("unreachable") } + +func cmpSpKeyName(p *StaticProperty, name any) int { + return util.Compare(p.Key, name) +} + +func cmpSpKey(a, b *StaticProperty) int { + return util.Compare(a.Key, b.Key) +} diff --git a/v1/types/types_bench_test.go b/v1/types/types_bench_test.go index 7cbadbda96f..77e98115d9a 100644 --- a/v1/types/types_bench_test.go +++ b/v1/types/types_bench_test.go @@ -12,24 +12,34 @@ import ( ) func BenchmarkSelect(b *testing.B) { - sizes := []int{1000, 10000, 100000} + sizes := []int{1000} for _, size := range sizes { b.Run(strconv.Itoa(size), func(b *testing.B) { tpe := generateType(size) - runSelectBenchmark(b, tpe, json.Number(strconv.Itoa(size-1))) + var num any = json.Number(strconv.Itoa(size - 1)) + for b.Loop() { + if result := Select(tpe, num); result != nil { + if Compare(result, N) != 0 { + b.Fatal("expected number type") + } + } + } }) } } -func runSelectBenchmark(b *testing.B, tpe Type, key any) { - - for b.Loop() { - if result := Select(tpe, key); result != nil { - if Compare(result, N) != 0 { - b.Fatal("expected number type") - } +func generateTypes(n int, prefix ...string) Any { + types := make([]Type, 0, n) + if len(prefix) > 0 { + for i := range n { + types = append(types, generateTypeWithPrefix(i, prefix[0])) + } + } else { + for i := range n { + types = append(types, generateType(i)) } } + return types } func generateType(n int) Type { @@ -51,11 +61,7 @@ func generateTypeWithPrefix(n int, prefix string) Type { func BenchmarkAnyMergeOne(b *testing.B) { sizes := []int{100, 500, 1000, 5000, 10000} for _, size := range sizes { - anyA := Any(make([]Type, 0, size)) - for i := range size { - tpe := generateType(i) - anyA = append(anyA, tpe) - } + anyA := generateTypes(size) tpeB := N b.Run(strconv.Itoa(size), func(b *testing.B) { for b.Loop() { @@ -73,16 +79,8 @@ func BenchmarkAnyUnionAllUniqueTypes(b *testing.B) { sizes := []int{100, 250, 500, 1000, 2500} for _, sizeA := range sizes { for _, sizeB := range sizes { - anyA := Any(make([]Type, 0, sizeA)) - for i := range sizeA { - tpe := generateType(i) - anyA = append(anyA, tpe) - } - anyB := Any(make([]Type, 0, sizeB)) - for i := range sizeB { - tpe := generateTypeWithPrefix(i, "B-") - anyB = append(anyB, tpe) - } + anyA := generateTypes(sizeA) + anyB := generateTypes(sizeB, "B-") b.Run(fmt.Sprintf("%dx%d", sizeA, sizeB), func(b *testing.B) { for b.Loop() { resultA2B := anyA.Union(anyB) diff --git a/v1/util/compare.go b/v1/util/compare.go index ec12210c6e3..8d0ef042776 100644 --- a/v1/util/compare.go +++ b/v1/util/compare.go @@ -164,6 +164,15 @@ func Compare(a, b any) int { } func compareJSONNumber(a, b json.Number) int { + if a == b { + return 0 + } + if ai, ok := Atoi(string(a)); ok { + if bi, ok := Atoi(string(b)); ok { + return ai - bi + } + return -1 + } bigA, ok := new(big.Float).SetString(string(a)) if !ok { panic("illegal value") diff --git a/v1/util/performance.go b/v1/util/performance.go index 2e44a0b30dd..ffb9fdb1f77 100644 --- a/v1/util/performance.go +++ b/v1/util/performance.go @@ -2,6 +2,7 @@ package util import ( "bytes" + "cmp" "encoding" "io" "slices" @@ -302,3 +303,9 @@ func SortedFunc[T any, S ~[]T](s S, cmp func(a, b T) int) S { slices.SortFunc(s, cmp) return s } + +// Sorted is simply a shorthand for [slices.Sort] which also returns the sorted slice. +func Sorted[T cmp.Ordered, S ~[]T](s S) S { + slices.Sort(s) + return s +}