Skip to content

Commit 2306cd6

Browse files
anderseknertanderseknert
committed
perf: avoid JSON/map roundtrips in ast.Value transforms
Leveraging the new capabilities for this in roast, we can avoid a lot of allocations by mapping directly from a source to an ast.Value rather than going via a `map[string]any`, or even JSON. For now, focus has been on how the **input** is mapped in most places of eval. This can be improved further, but the natural next step is of course to eliminate roundtrips the same way when we handle **output**, i.e. the result of evaluation. I imagine the data-heavy aggregate rules in particular could benefit greatly from this. This PR also cleans up some in LSP handlers related to input transformations, and takes a small step towards harmonizing the input format for those with the goal of eventually having a single schema to describe the common parts of all LSP request payloads. Perf impact is close to 2.9 million allocs less in the Regal lint benchmark, and greatly reduced ns/op and B/op. ``` 944216438 ns/op 2805943268 B/op 51659258 allocs/op - main 894474146 ns/op 2668457832 B/op 48779590 allocs/op - PR ``` Signed-off-by: Anders Eknert <anders@styra.com>
1 parent 25b1072 commit 2306cd6

28 files changed

Lines changed: 220 additions & 339 deletions

File tree

‎bundle/regal/lsp/clients/clients.rego‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
# METADATA
2+
# description: Client identifiers known by the Regal LSP server
13
package regal.lsp.clients
24

35
# METADATA

‎bundle/regal/lsp/completion/providers/packagename/packagename.rego‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ items contains item if {
1616
startswith(line, "package ")
1717
position.character > 7
1818

19-
ps := input.regal.context.path_separator
19+
ps := input.regal.environment.path_separator
2020

2121
abs_dir := _base(input.regal.file.name)
2222
rel_dir := trim_prefix(abs_dir, input.regal.context.workspace_root)

‎bundle/regal/lsp/completion/providers/packagename/packagename_test.rego‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@ test_package_name_completion_on_typing if {
1010
"lines": split(policy, "\n"),
1111
},
1212
"context": {
13-
"path_separator": "/",
1413
"workspace_root": "/Users/joe/policy",
1514
"location": {
1615
"row": 1,
1716
"col": 10,
1817
},
1918
},
19+
"environment": {"path_separator": "/"},
2020
}}
2121
items := provider.items with input as provider_input
2222
items == {{
@@ -41,13 +41,13 @@ test_package_name_completion_on_typing_multiple_suggestions if {
4141
"lines": split(policy, "\n"),
4242
},
4343
"context": {
44-
"path_separator": "/",
4544
"workspace_root": "/Users/joe/policy",
4645
"location": {
4746
"row": 1,
4847
"col": 10,
4948
},
5049
},
50+
"environment": {"path_separator": "/"},
5151
}}
5252
items := provider.items with input as provider_input
5353
items == {
@@ -86,13 +86,13 @@ test_package_name_completion_on_typing_multiple_suggestions_when_invoked if {
8686
"lines": split(policy, "\n"),
8787
},
8888
"context": {
89-
"path_separator": "/",
9089
"workspace_root": "/Users/joe/policy",
9190
"location": {
9291
"row": 1,
9392
"col": 9,
9493
},
9594
},
95+
"environment": {"path_separator": "/"},
9696
}}
9797
items := provider.items with input as provider_input
9898
items == {

‎bundle/regal/lsp/completion/providers/regov1/regov1.rego‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ import data.regal.lsp.completion.kind
88
import data.regal.lsp.completion.location
99

1010
# METADATA
11-
# description: completion suggestion for rego.v1
11+
# description: completion suggestion for rego.v1 import
1212
items contains item if {
13-
input.regal.context.rego_version != 3 # the rego.v1 import is not used in v1 rego (3)
13+
input.regal.file.rego_version != "v1" # the rego.v1 import is not used in v1 Rego
1414
not strings.any_prefix_match(input.regal.file.lines, "import rego.v1")
1515

1616
position := location.to_position(input.regal.context.location)

‎bundle/regal/lsp/completion/providers/regov1/regov1_test.rego‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ test_regov1_completion_on_typing if {
88
policy := `package policy
99
1010
import r`
11-
items := provider.items with input as util.input_with_location_and_version(policy, {"row": 3, "col": 9}, 0)
11+
items := provider.items with input as util.input_with_location_and_version(policy, {"row": 3, "col": 9}, "v0")
1212
items == {{
1313
"label": "rego.v1",
1414
"kind": 9,
@@ -33,7 +33,7 @@ test_regov1_completion_on_invoked if {
3333
policy := `package policy
3434
3535
import `
36-
items := provider.items with input as util.input_with_location_and_version(policy, {"row": 3, "col": 8}, 0)
36+
items := provider.items with input as util.input_with_location_and_version(policy, {"row": 3, "col": 8}, "v0")
3737
items == {{
3838
"label": "rego.v1",
3939
"kind": 9,
@@ -60,7 +60,7 @@ test_no_regov1_completion_if_already_imported if {
6060
import rego.v1
6161
6262
import r`
63-
items := provider.items with input as util.input_with_location_and_version(policy, {"row": 5, "col": 9}, 0)
63+
items := provider.items with input as util.input_with_location_and_version(policy, {"row": 5, "col": 9}, "v0")
6464
items == set()
6565
}
6666

@@ -71,7 +71,7 @@ import r`
7171
items := provider.items with input as util.input_with_location_and_version(
7272
policy,
7373
{"row": 3, "col": 9},
74-
3, # RegoV1
74+
"v1", # RegoV1
7575
)
7676
items == set()
7777
}

‎bundle/regal/lsp/completion/providers/test_utils/test_utils.rego‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ input_with_location_and_version(policy, location, rego_version) := {"regal": {
3535
"file": {
3636
"name": "p.rego",
3737
"lines": split(policy, "\n"),
38-
},
39-
"context": {
40-
"location": location,
4138
"rego_version": rego_version,
4239
},
40+
"context": {"location": location},
4341
}}

‎cmd/test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ func Runtime() *ast.Term {
397397
for _, s := range os.Environ() {
398398
parts := strings.SplitN(s, "=", 2)
399399
if len(parts) == 1 {
400-
env.Insert(ast.StringTerm(parts[0]), ast.NullTerm())
400+
env.Insert(ast.StringTerm(parts[0]), ast.InternedNullTerm)
401401
} else if len(parts) > 1 {
402402
env.Insert(ast.StringTerm(parts[0]), ast.StringTerm(parts[1]))
403403
}

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ require (
2222
github.com/sourcegraph/jsonrpc2 v0.2.1
2323
github.com/spf13/cobra v1.9.1
2424
github.com/spf13/pflag v1.0.6
25-
github.com/styrainc/roast v0.12.0
25+
github.com/styrainc/roast v0.14.0
2626
gopkg.in/yaml.v3 v3.0.1
2727
)
2828

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,8 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
305305
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
306306
github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
307307
github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
308-
github.com/styrainc/roast v0.12.0 h1:TCxazAd1PodsmzMlWgaomDxsx48QdFqJWGldxC7YEpg=
309-
github.com/styrainc/roast v0.12.0/go.mod h1:cKELz96vKP1jeA/0HeyMW6gOqvDI6eo3wmq5/a43TYA=
308+
github.com/styrainc/roast v0.14.0 h1:UewC6U8A2MvUUnXRQggHFvpr94jSHzFEvcyYVfbLutc=
309+
github.com/styrainc/roast v0.14.0/go.mod h1:cKELz96vKP1jeA/0HeyMW6gOqvDI6eo3wmq5/a43TYA=
310310
github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8=
311311
github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU=
312312
github.com/tchap/go-patricia/v2 v2.3.2 h1:xTHFutuitO2zqKAQ5rCROYgUb7Or/+IC3fts9/Yc7nM=

‎internal/lsp/clients/clients.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package clients
22

33
// Identifier represent different supported clients and can be used to toggle or change
44
// server behavior based on the client.
5-
type Identifier int
5+
type Identifier uint8
66

77
const (
88
IdentifierGeneric Identifier = iota

0 commit comments

Comments
 (0)