Skip to content

Commit

Permalink
Roast v0.5.0, faster input conversion (StyraInc#1293)
Browse files Browse the repository at this point in the history
Using the new `ToOPAInput` function from RoAST cuts the number
of allocations related to input transformation in half.

This doesn't constitute a bottleneck for command line linting,
but will help the language server process files as quickly as
possible on changes.

Also convert a few bytes/string conversions to allocation-free
variants. I don't think these matter much, but having them around
will help remind us that's an option for when it does matter.

Signed-off-by: Anders Eknert <anders@styra.com>
  • Loading branch information
anderseknert authored and charlieegan3 committed Jan 6, 2025
1 parent f14cac7 commit ae5a215
Show file tree
Hide file tree
Showing 30 changed files with 141 additions and 110 deletions.
3 changes: 2 additions & 1 deletion cmd/debugger.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"os"
"strings"

"github.com/anderseknert/roast/pkg/util"
godap "github.com/google/go-dap"
"github.com/spf13/cobra"

Expand Down Expand Up @@ -354,7 +355,7 @@ func pos(loc *location.Location) (source *godap.Source, line, col, endLine, endC
}
}

lines := strings.Split(string(loc.Text), "\n")
lines := strings.Split(util.ByteSliceToString(loc.Text), "\n")
line = loc.Row
col = loc.Col

Expand Down
7 changes: 4 additions & 3 deletions cmd/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"

"github.com/anderseknert/roast/pkg/encoding"
"github.com/anderseknert/roast/pkg/util"
"github.com/spf13/cobra"

"github.com/open-policy-agent/opa/ast"
Expand Down Expand Up @@ -51,7 +52,7 @@ func parse(args []string) error {
return err
}

content := string(bs)
content := util.ByteSliceToString(bs)

module, err := ast.ParseModuleWithOpts(filename, content, rp.ParserOptions())
if err != nil {
Expand All @@ -63,12 +64,12 @@ func parse(args []string) error {
return err
}

bs, err = encoding.JSON().MarshalIndent(enhancedAST, "", " ")
output, err := encoding.JSON().MarshalIndent(enhancedAST, "", " ")
if err != nil {
return err
}

_, err = os.Stdout.Write(bs)
_, err = os.Stdout.Write(output)

return err
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.22.5

require (
dario.cat/mergo v1.0.1
github.com/anderseknert/roast v0.4.2
github.com/anderseknert/roast v0.5.0
github.com/coreos/go-semver v0.3.1
github.com/fatih/color v1.18.0
github.com/fsnotify/fsnotify v1.8.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ github.com/ProtonMail/go-crypto v1.0.0 h1:LRuvITjQWX+WIfr930YHG2HNfjR1uOfyf5vE0k
github.com/ProtonMail/go-crypto v1.0.0/go.mod h1:EjAoLdwvbIOoOQr3ihjnSoLZRtE8azugULFRteWMNc0=
github.com/agnivade/levenshtein v1.2.0 h1:U9L4IOT0Y3i0TIlUIDJ7rVUziKi/zPbrJGaFrtYH3SY=
github.com/agnivade/levenshtein v1.2.0/go.mod h1:QVVI16kDrtSuwcpd0p1+xMC6Z/VfhtCyDIjcwga4/DU=
github.com/anderseknert/roast v0.4.2 h1:RQLxE2IcqjpXnM/M2qlCrt8bkp6Rg1JCbbTOfjvDxmM=
github.com/anderseknert/roast v0.4.2/go.mod h1:LJIt906EwHkwKAOePJIY5DUuDumHto0xryuLjR4Nq5A=
github.com/anderseknert/roast v0.5.0 h1:2TJ3kX+OCeesArnK3ieRmMFf9nLrWYfVeIR0Ol9mon0=
github.com/anderseknert/roast v0.5.0/go.mod h1:9RXWYE5aTdo1Ro1FZzf26shaPUMm2fH0ibKrY9DH224=
github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be h1:9AeTilPcZAjCFIImctFaOjnTIavg87rW78vTPkQqLI8=
github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be/go.mod h1:ySMOLuWl6zY27l47sB3qLNK6tF2fkHG55UZxx8oIVo4=
github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo=
Expand Down
3 changes: 2 additions & 1 deletion internal/explorer/stages.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"

"github.com/anderseknert/roast/pkg/encoding"
"github.com/anderseknert/roast/pkg/util"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/bundle"
Expand Down Expand Up @@ -133,7 +134,7 @@ func Plan(ctx context.Context, path, rego string, usePrint bool) (string, error)
{
URL: "/url",
Path: path,
Raw: []byte(rego),
Raw: util.StringToByteSlice(rego),
Parsed: mod,
},
},
Expand Down
23 changes: 2 additions & 21 deletions internal/io/io.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,30 +57,11 @@ func MustLoadRegalBundleFS(fs files.FS) bundle.Bundle {
func ToMap(a any) map[string]any {
r := make(map[string]any)

MustJSONRoundTrip(a, &r)
encoding.MustJSONRoundTrip(a, &r)

return r
}

// JSONRoundTrip convert any value to JSON and back again.
func JSONRoundTrip(from any, to any) error {
json := encoding.JSON()

bs, err := json.Marshal(from)
if err != nil {
return fmt.Errorf("failed JSON marshalling %w", err)
}

return json.Unmarshal(bs, to) //nolint:wrapcheck
}

// MustJSONRoundTrip convert any value to JSON and back again, exit on failure.
func MustJSONRoundTrip(from any, to any) {
if err := JSONRoundTrip(from, to); err != nil {
log.Fatal(err)
}
}

// CloseFileIgnore closes file ignoring errors, mainly for deferred cleanup.
func CloseFileIgnore(file *os.File) {
_ = file.Close()
Expand All @@ -102,7 +83,7 @@ func ExcludeTestFilter() filter.LoaderFilter {
// - While the input data theoritcally could be anything JSON/YAML value, we only support an object.
func FindInput(file string, workspacePath string) (string, map[string]any) {
relative := strings.TrimPrefix(file, workspacePath)
components := strings.Split(filepath.Dir(relative), string(filepath.Separator))
components := strings.Split(filepath.Dir(relative), PathSeparator)

var (
inputPath string
Expand Down
19 changes: 0 additions & 19 deletions internal/io/io_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,6 @@ import (
"github.com/open-policy-agent/opa/util/test"
)

func TestJSONRoundTrip(t *testing.T) {
t.Parallel()

type foo struct {
Bar string `json:"bar"`
}

m := map[string]any{"bar": "foo"}
f := foo{}

if err := JSONRoundTrip(m, &f); err != nil {
t.Fatal(err)
}

if f.Bar != "foo" {
t.Errorf("expected JSON roundtrip to set struct value")
}
}

func TestFindManifestLocations(t *testing.T) {
t.Parallel()

Expand Down
4 changes: 2 additions & 2 deletions internal/lsp/bundles/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ type CacheOptions struct {
func NewCache(opts *CacheOptions) *Cache {
workspacePath := opts.WorkspacePath

if !strings.HasSuffix(workspacePath, string(filepath.Separator)) {
workspacePath += string(filepath.Separator)
if !strings.HasSuffix(workspacePath, rio.PathSeparator) {
workspacePath += rio.PathSeparator
}

c := &Cache{
Expand Down
4 changes: 3 additions & 1 deletion internal/lsp/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"os"
"sync"

"github.com/anderseknert/roast/pkg/util"

"github.com/open-policy-agent/opa/ast"

"github.com/styrainc/regal/internal/lsp/types"
Expand Down Expand Up @@ -388,7 +390,7 @@ func UpdateCacheForURIFromDisk(cache *Cache, fileURI, path string) (bool, string
return false, "", fmt.Errorf("failed to read file: %w", err)
}

currentContent := string(content)
currentContent := util.ByteSliceToString(content)

cachedContent, ok := cache.GetFileContents(fileURI)
if ok && cachedContent == currentContent {
Expand Down
7 changes: 4 additions & 3 deletions internal/lsp/completions/providers/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import (
"context"
"errors"
"fmt"
"os"

"github.com/anderseknert/roast/pkg/encoding"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/rego"
Expand Down Expand Up @@ -65,7 +66,7 @@ func (p *Policy) Run(
}
inputContext["client_identifier"] = opts.ClientIdentifier
inputContext["workspace_root"] = uri.ToPath(opts.ClientIdentifier, opts.RootURI)
inputContext["path_separator"] = string(os.PathSeparator)
inputContext["path_separator"] = rio.PathSeparator

workspacePath := uri.ToPath(opts.ClientIdentifier, opts.RootURI)

Expand Down Expand Up @@ -97,7 +98,7 @@ func (p *Policy) Run(

completions := make([]types.CompletionItem, 8)

if err = rio.JSONRoundTrip(result["completions"], &completions); err != nil {
if err := encoding.JSONRoundTrip(result["completions"], &completions); err != nil {
return nil, fmt.Errorf("failed converting completions: %w", err)
}

Expand Down
7 changes: 3 additions & 4 deletions internal/lsp/completions/providers/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@ import (
"slices"
"testing"

"github.com/anderseknert/roast/pkg/encoding"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/storage/inmem"

rio "github.com/styrainc/regal/internal/io"
"github.com/styrainc/regal/internal/lsp/cache"
"github.com/styrainc/regal/internal/lsp/clients"
"github.com/styrainc/regal/internal/lsp/types"
"github.com/styrainc/regal/internal/parse"

_ "github.com/anderseknert/roast/pkg/encoding"
)

func TestPolicyProvider_Example1(t *testing.T) {
Expand All @@ -35,7 +34,7 @@ allow if {

moduleMap := make(map[string]any)

rio.MustJSONRoundTrip(module, &moduleMap)
encoding.MustJSONRoundTrip(module, &moduleMap)

c.SetFileContents(testCaseFileURI, policy)

Expand Down
6 changes: 4 additions & 2 deletions internal/lsp/completions/refs/defined.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package refs

import (
"bytes"
"fmt"
"slices"
"strings"

"github.com/anderseknert/roast/pkg/util"
"gopkg.in/yaml.v3"

"github.com/open-policy-agent/opa/ast"
Expand Down Expand Up @@ -227,11 +229,11 @@ func documentAnnotatedRef(selectedAnnotation *ast.Annotations) string {
if len(selectedAnnotation.Custom) > 0 {
sb.WriteString("**Custom**:\n\n```yaml\n")

yamlBs, err := yaml.Marshal(selectedAnnotation.Custom)
bs, err := yaml.Marshal(selectedAnnotation.Custom)
if err != nil {
sb.WriteString("Error generating custom section")
} else {
sb.WriteString(strings.TrimSpace(string(yamlBs)))
sb.WriteString(util.ByteSliceToString(bytes.TrimSpace(bs)))
}

sb.WriteString("\n```\n")
Expand Down
9 changes: 8 additions & 1 deletion internal/lsp/completions/refs/used.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"sync"

"github.com/anderseknert/roast/pkg/transform"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/bundle"
"github.com/open-policy-agent/opa/rego"
Expand Down Expand Up @@ -69,7 +71,12 @@ func initialize() {
func UsedInModule(ctx context.Context, module *ast.Module) ([]string, error) {
pqInitOnce.Do(initialize)

rs, err := pq.Eval(ctx, rego.EvalInput(module))
inputValue, err := transform.ToOPAInputValue(module)
if err != nil {
return nil, fmt.Errorf("failed converting input to value: %w", err)
}

rs, err := pq.Eval(ctx, rego.EvalParsedInput(inputValue))
if err != nil {
return nil, fmt.Errorf("failed to evaluate rego query: %w", err)
}
Expand Down
12 changes: 11 additions & 1 deletion internal/lsp/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"fmt"
"strings"

"github.com/anderseknert/roast/pkg/transform"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/bundle"
"github.com/open-policy-agent/opa/rego"
Expand Down Expand Up @@ -79,13 +81,21 @@ func (l *LanguageServer) Eval(
l.getLoadedConfig(),
)

// TODO: Let's try to avoid preparing on each eval, but only when the
// contents of the workspace modules change, and before the user requests
// an eval.
pq, err := rego.New(regoArgs...).PrepareForEval(ctx)
if err != nil {
return nil, fmt.Errorf("failed preparing query %s: %w", query, err)
}

if input != nil {
return pq.Eval(ctx, rego.EvalInput(input)) //nolint:wrapcheck
inputValue, err := transform.ToOPAInputValue(input)
if err != nil {
return nil, fmt.Errorf("failed converting input to value: %w", err)
}

return pq.Eval(ctx, rego.EvalParsedInput(inputValue)) //nolint:wrapcheck
}

return pq.Eval(ctx) //nolint:wrapcheck
Expand Down
4 changes: 3 additions & 1 deletion internal/lsp/opa/oracle/oracle.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package oracle
import (
"errors"

"github.com/anderseknert/roast/pkg/util"

"github.com/open-policy-agent/opa/ast"

"github.com/styrainc/regal/internal/compile"
Expand Down Expand Up @@ -157,7 +159,7 @@ func compileUpto(stage string, modules map[string]*ast.Module, bs []byte, filena

if len(bs) > 0 {
var err error
module, err = ast.ParseModule(filename, string(bs))
module, err = ast.ParseModule(filename, util.ByteSliceToString(bs))
if err != nil {
return nil, nil, err
}
Expand Down
12 changes: 7 additions & 5 deletions internal/lsp/opa/scanner/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"unicode"
"unicode/utf8"

"github.com/anderseknert/roast/pkg/util"

"github.com/styrainc/regal/internal/lsp/opa/tokens"
)

Expand Down Expand Up @@ -267,7 +269,7 @@ func (s *Scanner) scanIdentifier() string {
for isLetter(s.curr) || isDigit(s.curr) {
s.next()
}
return string(s.bs[start : s.offset-1])
return util.ByteSliceToString(s.bs[start : s.offset-1])
}

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

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

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

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

func (s *Scanner) scanRawString() string {
Expand All @@ -366,7 +368,7 @@ func (s *Scanner) scanRawString() string {
break
}
}
return string(s.bs[start : s.offset-1])
return util.ByteSliceToString(s.bs[start : s.offset-1])
}

func (s *Scanner) scanComment() string {
Expand All @@ -379,7 +381,7 @@ 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 util.ByteSliceToString(s.bs[start:end])
}

func (s *Scanner) next() {
Expand Down
Loading

0 comments on commit ae5a215

Please # to comment.