Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize biunify for term slices #7168

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

anderseknert
Copy link
Member

@anderseknert anderseknert commented Nov 11, 2024

The construction of intermediate throwaway ast.Arrays caused almost 5 million allocations when running regal lint against its own bundle.

While the results are likely more impressive for Regal policies than the average one, this is still a substantial improvement across a large group of policies.

regal lint OPA main

BenchmarkRegalLintingItself-10    1	3317838417 ns/op	6611412800 B/op	124873123 allocs/op

regal lint this branch

BenchmarkRegalLintingItself-10    1	3140384416 ns/op	6590159176 B/op	120098613 allocs/op

ast/term.go Outdated Show resolved Hide resolved
srenatus
srenatus previously approved these changes Nov 11, 2024
The construction of intermediate throwaway `ast.Array`s caused almost
5 million allocations when running `regal lint` against its own bundle.

While the results are likely more impressive for Regal policies than the
average one, this is still a substantial improvement across a large group
of policies.

**regal lint OPA main**
```
BenchmarkRegalLintingItself-10    1	3317838417 ns/op	6611412800 B/op	124873123 allocs/op
```

**regal lint this branch**
```
BenchmarkRegalLintingItself-10    1	3140384416 ns/op	6590159176 B/op	120098613 allocs/op
```

Signed-off-by: Anders Eknert <anders@styra.com>
@srenatus srenatus changed the title Optimize biunify for arrays Optimize biunify for term slices Nov 11, 2024
@anderseknert anderseknert merged commit c0fe200 into open-policy-agent:main Nov 11, 2024
28 checks passed
@anderseknert anderseknert deleted the eval-less-allocs branch November 11, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants