- Notifications
You must be signed in to change notification settings - Fork928
chore: Optimize rego policy input allocations#6135
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Manually convert to ast.Value instead of using genericjson.Marshal conversion.
The optimized input is always compared to the normal jsonmarshal parser.
Uh oh!
There was an error while loading.Please reload this page.
| @@ -79,6 +80,8 @@ var ( | |||
| Site: permissions(map[string][]Action{ | |||
| ResourceWildcard.Type: {WildcardSymbol}, | |||
| }), | |||
| Org: map[string][]Permission{}, | |||
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.
review: these had been breaking comparison due to comparing empty vs nil slice
| // Currently ast.Object.insert() is the slowest part of the process and allocates | ||
| // the most amount of bytes. This general approach copies all of our struct | ||
| // data and uses a lot of extra memory for handling things like sort order. | ||
| // A possible large improvement would be to implement the ast.Value interface directly. |
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.
👍
Uh oh!
There was an error while loading.Please reload this page.
Manually convert to ast.Value instead of using generic json.Marshal conversion.
Saves ~0.1ms from all users with roles + groups.
Tests
The test
TestRegoInputValueensures there is no difference in the optimized output to the prior json.Marshal method. So this PR is 100% safe! 🥳Results
Input allocations
This is the benchmark of just the saved allocations on the inputs.
JSONRegoValuewas the previous method.ManualRegoValueis new technique.33% reduction in the number of bytes allocated for a rather complex rbac subject. Actual savings depends on the actor and things like the number of groups they are in.
Broader RBAC benchmark impact
Time savings are measurable! Not an order of magnitude, but it is faster.
Before
After
Future work
If we implement
ast.Valueinterface directly for our maps (eg roles), we can reduce a lot more allocations.