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

perf: move config check out of loop #1351

Merged
merged 1 commit into from
Jan 20, 2025
Merged

perf: move config check out of loop #1351

merged 1 commit into from
Jan 20, 2025

Conversation

anderseknert
Copy link
Member

In main.rego there are two places where we extract a relative path from the input policy file. This was however done in a loop despite only using variables from outside of the loop. Turns out there was close to a million allocations hiding there, lol.

BenchmarkRegalLintingItself-10 Before

1875658375 ns/op    3539151104 B/op    70020076 allocs/op

BenchmarkRegalLintingItself-10 After

1821361667 ns/op    3499082704 B/op    69062781 allocs/op

Copy link
Member

@charlieegan3 charlieegan3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍

In main.rego there are two places where we extract a relative path from
the input policy file. This was however done in a loop despite only using
variables from outside of the loop. Turns out there was close to a million
allocations hiding there, lol.

**BenchmarkRegalLintingItself-10 Before**
```
1875658375 ns/op    3539151104 B/op    70020076 allocs/op
```
**BenchmarkRegalLintingItself-10 After**
```
1821361667 ns/op    3499082704 B/op    69062781 allocs/op
```

Signed-off-by: Anders Eknert <[email protected]>
@anderseknert anderseknert merged commit 7b693ee into main Jan 20, 2025
5 checks passed
@anderseknert anderseknert deleted the minor-perf-fix branch January 20, 2025 15:58
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