25% ns/op reduction by aggregate remodel + Rego prepare stage#1838
25% ns/op reduction by aggregate remodel + Rego prepare stage#1838anderseknert merged 1 commit intoopen-policy-agent:mainfrom
Conversation
srenatus
left a comment
srenatus
left a comment
There was a problem hiding this comment.
Skimmed most of the go code! LGTM 🚀
anderseknert
force-pushed
the
aggregate-ast
branch
from
de0574e to
7ac6335
Compare
| file_notices if input.regal.file.rego_version != "v1" | ||
| file_notices if count(config.capabilities.special.no_filename) > 0 |
There was a problem hiding this comment.
A little more of a comment about these two cases might help in future. Even just a reminder that no_filename is stdin, and therefore we can't lookup.
|
Nice work here, ogre is a nice abstraction for the project! |
|
Running the tests repeatedly on my laptop and I'm not able to reproduce any of the issues that fail the CI build. I'll look some more into it. They're all related to the language server, and I have mostly focused on the linter here. Could be that there's some more work needed to have them play nicely together. But it's hard when you can't reproduce the issue locally :( |
anderseknert
force-pushed
the
aggregate-ast
branch
2 times, most recently
from
4fcbb2f to
a330dde
Compare
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 open-policy-agent#359 Fixes open-policy-agent#1265 And probably a few more, lol Signed-off-by: Anders Eknert <anders.eknert@apple.com>
anderseknert
force-pushed
the
aggregate-ast
branch
from
a330dde to
1785e5d
Compare
|
Oh, I had forgotten to revert the change of the default timeout for the LSP tests from 20 seconds to 2. That could explain a thing or two... :) I have found it quite useful to use a much lower value for local development, as issues tend to surface much quicker that way. So I'll see about having something that allows keeping 20 seconds in CI while setting it lower locally. |
3 participants
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 :)
ogrepackage for lightweightregoalternativeFixes #359
Fixes #1265
And probably a few more, lol