-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Template string performance improvements and more #8143
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
Template string performance improvements and more #8143
Conversation
| // BenchmarkBuiltinTemplateString/primitives-16 8217890 144.9 ns/op 440 B/op 7 allocs/op | ||
| // BenchmarkBuiltinTemplateString/collections-16 2056494 583.7 ns/op 1144 B/op 28 allocs/op | ||
| // BenchmarkBuiltinTemplateString/multiple_outputs-16 9424003 128.8 ns/op 480 B/op 7 allocs/op | ||
| func BenchmarkBuiltinTemplateString(b *testing.B) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this PR:
// BenchmarkBuiltinTemplateString/no_parts-16 8326574 128.5 ns/op 632 B/op 5 allocs/op
// BenchmarkBuiltinTemplateString/single_string_part-16 6282360 189.5 ns/op 952 B/op 8 allocs/op
// BenchmarkBuiltinTemplateString/single_undefined_part-16 6194984 192.3 ns/op 952 B/op 8 allocs/op
// BenchmarkBuiltinTemplateString/primitives-16 3178212 379.5 ns/op 1880 B/op 12 allocs/op
// BenchmarkBuiltinTemplateString/collections-16 1000000 1241 ns/op 4603 B/op 40 allocs/op
// BenchmarkBuiltinTemplateString/multiple_outputs-16 4449758 269.4 ns/op 1344 B/op 10 allocs/op
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
srenatus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
There's a conflict.... Please take a look 👇 |
A mixed bag of improvements I have had around for a while, and would like to see included in v1.12.0 if possible. I have about twice the amount of changes **not** included here as they could use some more testing. These changes should be low risk, I believe.. but obviously do let me know if you see any potential risks that I don't! - Add template string benchmarks - Faster template strings / print eval by not passing bctx in recursion - Allow passing nil value to `Query.WithQueryTracer` (no-op) - Reduce allocations in rego v1 compiler stages - Intern a few more common var name `Value`s - Remove redundant switch on `scope` in annotations code - Add a few more benchmarks in the `ast` package - Performance improvements in type checker, most notably removing a function literal for checking expression, which only ever had one implementation. We can extend this later if needed. - Prefer `NewGenericTransformer` over `&GenericTransformer` for easier tracking in pprof Signed-off-by: Anders Eknert <[email protected]>
34dfc32 to
b53f81b
Compare
Signed-off-by: Johan Fylling <[email protected]>
A mixed bag of improvements I have had around for a while, and would like to see included in v1.12.0 if possible. I have about twice the amount of changes not included here as they could use some more testing. These changes should be low risk, I believe.. but obviously do let me know if you see any potential risks that I don't!
Query.WithQueryTracer(no-op)Valuesscopein annotations codeastpackageNewGenericTransformerover&GenericTransformerfor easier tracking in pprof