Skip to content

Commit 698eeba

Browse files
authored
25% ns/op reduction by aggregate remodel + Rego prepare stage (#1838)
This concludes work I started before the holidays, and which I have worked on when given the time. It's a lot! Sorry about that, but it'll be worth it, I'm sure :) - Remodel aggregate collection to keep AST shape without roundtrips - Remove all metadata on each aggregate item for performance - Introduce `ogre` package for lightweight `rego` alternative - Introduce new prepare stage done in Rego, where we set up things once and persist to store instead of in every file linted Fixes #359 Fixes #1265 And probably a few more, lol Signed-off-by: Anders Eknert <anders.eknert@apple.com>
1 parent 38e621c commit 698eeba

55 files changed

Lines changed: 2139 additions & 1465 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

bundle/regal/ast/ast.rego

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ package_path := [path.value |
7979
# input policy, so "package foo.bar" would return "foo.bar"
8080
package_name := concat(".", package_path)
8181

82+
# METADATA
83+
# description: provides the full package name including the "data." prefix,
84+
package_name_full := concat("", ["data.", package_name])
85+
8286
# METADATA
8387
# description: provides all static string values from ref
8488
named_refs(ref) := [term |

bundle/regal/main/main.rego

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,24 @@ package regal.main
1111
import data.regal.ast
1212
import data.regal.config
1313
import data.regal.notices
14+
import data.regal.prepared
1415
import data.regal.util
1516

1617
# METADATA
17-
# description: set of all notices returned from linter rules
18+
# description: |
19+
# set of all notices returned from linter rules
20+
# all notices for v1 projects run in the prepare stage, and we only run
21+
# the (rather expensive per-file notices if a file specifically is not v1
22+
# scope: document
1823
lint.notices contains notice if {
1924
"lint" in input.regal.operations
2025

26+
prepared.file_notices
27+
2128
some category, title
2229
_rules_to_run[category][title]
2330

24-
rule_notices := notices.promoted_notices[category][title]
25-
some notice in rule_notices
31+
some notice in notices.promoted_notices[category][title]
2632
}
2733

2834
# METADATA
@@ -41,6 +47,10 @@ lint.aggregates := aggregate if "collect" in input.regal.operations
4147
# description: all violations from aggregate rules
4248
lint.aggregate.violations := aggregate_report if "aggregate" in input.regal.operations
4349

50+
# METADATA
51+
# description: prepared state for linting, after Rego preparation step
52+
lint.prepared := prepared.prepare if "prepare" in input.regal.operations
53+
4454
_file_name_relative_to_root(filename, "/") := trim_prefix(filename, "/")
4555
_file_name_relative_to_root(filename, root) := trim_prefix(filename, concat("", [root, "/"])) if root != "/"
4656

@@ -60,9 +70,8 @@ _rules_to_run[category] contains title if {
6070
not config.ignored_globally(relative_filename)
6171

6272
some category, title
63-
config.rules[category][title]
73+
prepared.rules_to_run[category][title]
6474

65-
not config.ignored_rule(category, title)
6675
not config.excluded_file(category, title, relative_filename)
6776
}
6877

@@ -96,7 +105,7 @@ report contains violation if {
96105
some category, title
97106
_rules_to_run[category][title]
98107

99-
count(object.get(notices.promoted_notices, [category, title], [])) == 0
108+
count(object.get(prepared.notices, [category, title], [])) == 0
100109

101110
some violation in data.regal.rules[category][title].report
102111

@@ -109,7 +118,6 @@ report contains violation if {
109118
not config.ignored_globally(file_name_relative_to_root)
110119

111120
some category, title
112-
113121
violation := data.custom.regal.rules[category][title].report[_]
114122

115123
not config.ignored_rule(category, title)
@@ -120,19 +128,19 @@ report contains violation if {
120128
# METADATA
121129
# description: collects aggregates in bundled rules
122130
# scope: rule
123-
aggregate[category_title] contains entry if {
131+
aggregate[input.regal.file.name][category_title] contains entry if {
124132
some category, title
125133
_rules_to_run[category][title]
126134

127-
some entry in data.regal.rules[category][title].aggregate
135+
some entry in _mark_if_empty(data.regal.rules[category][title].aggregate)
128136

129137
category_title := concat("/", [category, title])
130138
}
131139

132140
# METADATA
133141
# description: collects aggregates in custom rules
134142
# scope: rule
135-
aggregate[category_title] contains entry if {
143+
aggregate[input.regal.file.name][category_title] contains entry if {
136144
not config.ignored_globally(input.regal.file.name)
137145

138146
some category, title
@@ -165,14 +173,7 @@ aggregate_report contains violation if {
165173
some category, title
166174
_rules_to_run[category][title]
167175

168-
key := concat("/", [category, title])
169-
input_for_rule := object.remove(
170-
object.union(input, {"aggregate": object.get(input, ["aggregates_internal", key], [])}),
171-
["aggregates_internal"],
172-
)
173-
174-
# regal ignore:with-outside-test-context
175-
some violation in data.regal.rules[category][title].aggregate_report with input as input_for_rule
176+
some violation in data.regal.rules[category][title].aggregate_report
176177

177178
not _ignored(violation, util.keys_to_numbers(object.get(
178179
input.ignore_directives,
@@ -187,18 +188,15 @@ aggregate_report contains violation if {
187188
# schemas:
188189
# - input: schema.regal.aggregate
189190
aggregate_report contains violation if {
190-
not config.ignored_globally(input.regal.file.name)
191+
data.custom.regal
191192

192-
some key in object.keys(input.aggregates_internal)
193+
some key, aggregate in _aggregate_report_inputs
193194
[category, title] := split(key, "/")
194195

195196
not config.ignored_rule(category, title)
196197
not config.excluded_file(category, title, input.regal.file.name)
197198

198-
input_for_rule := object.remove(
199-
object.union(input, {"aggregate": _null_to_empty(input.aggregates_internal[key])}),
200-
["aggregates_internal"],
201-
)
199+
input_for_rule := _remove_empty_aggregates(aggregate)
202200

203201
# regal ignore:with-outside-test-context
204202
some violation in data.custom.regal.rules[category][title].aggregate_report with input as input_for_rule
@@ -209,6 +207,29 @@ aggregate_report contains violation if {
209207
not _ignored(violation, util.keys_to_numbers(ignore_directives))
210208
}
211209

210+
_remove_empty_aggregates(aggregates) := {"aggregate": set()} if {
211+
aggregates == {"aggregate": {set()}}
212+
} else := aggregates
213+
214+
# METADATA
215+
# description: |
216+
# Restructure aggregate report input for conformance with "legacy"
217+
# format, so that we don't break existing rules. Later on we can deprecate
218+
# this and make it opt-in.
219+
# schemas:
220+
# - input: schema.regal.aggregate
221+
_aggregate_report_inputs[cat_title].aggregate contains formatted if {
222+
some filename, cat_title
223+
aggregate := input.aggregates_internal[filename][cat_title][_]
224+
formatted := _format_aggregate(aggregate, filename)
225+
}
226+
227+
default _format_aggregate(_, _) := set()
228+
229+
_format_aggregate(aggregate, filename) := object.union(aggregate, {"aggregate_source": {"file": filename}}) if {
230+
aggregate.aggregate_data
231+
}
232+
212233
_ignored(violation, directives) if {
213234
ignored_rules := directives[util.to_location_object(violation.location).row]
214235
violation.title in ignored_rules

0 commit comments

Comments
 (0)