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

Excessive time spent in ast.(*parser).parseExpr #78

Closed
briantkennedy opened this issue Feb 13, 2020 · 3 comments
Closed

Excessive time spent in ast.(*parser).parseExpr #78

briantkennedy opened this issue Feb 13, 2020 · 3 comments

Comments

@briantkennedy
Copy link
Contributor

I wrote a FCV benchmark test and profiled it using go test -cpuprofile cpu.prof -bench BenchmarkReviewJSON -benchtime 60s ./pkg/gcv and it looks like it's spending quite a bit of time in the opa AST parser. This seems odd since the rego is only added during benchmark setup.

benchmark

@patrick-east
Copy link

patrick-east commented Feb 14, 2020

A quick look at where the OPA eval happens (I think? Correct me if I'm wrong on this): https://github.com/forseti-security/config-validator/blob/258b9eb9e8e6d6e91da8edbcab771d19ece3fad8/pkg/gcv/cf/cf.go#L214-L222 it appears that the query is passed in as a string for each evaluation. If the actual eval is fast it wouldn't be super surprising to see that a large chunk of time is spent parsing the query on each of the benchmark loop iterations. I see the topdown.eval entries only taking up a few % on that screenshot so it seems likely that this is the case, especially if the iteration count was large for the benchmark.

More recently in OPA we have added a set of new API's on the rego package that prepares the rego.Rego object as a "prepared query" and can then be re-used for evaluations that eliminate the need to do the query prep (and a number of other things, depending on the options passed in). You can then use it with different inputs for evaluations and save a bunch of time, especially on repeated queries where only input is changing (which it looks like in this case here). Check out https://pkg.go.dev/github.com/open-policy-agent/opa@v0.17.1/rego?tab=doc#example-Rego.PrepareForEval

Again, just from a quick look. I might have totally missed some other spot that is doing additional parsing.

@patrick-east
Copy link

@maxsmythe, as discussed, to parse the query ahead of time you'd use https://godoc.org/github.com/open-policy-agent/opa/ast#ParseBody and then use this option to pass in the parsed query https://godoc.org/github.com/open-policy-agent/opa/rego#ParsedQuery

@willbeason
Copy link
Member

This has been resolved with #202 and #206 . We only parse Template rego once, and we don't recompile already existing Templates.

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

No branches or pull requests

3 participants