Performance impact switching to `some .. in` iteration syntax

[davidmarne-wf]

Short description

There appears to be situations where some .. in iteration syntax is less performant than the legacy bracket syntax.

Examples:

all_operations contains operation if {
	role_allowed_operations[_][operation]
}

all_operations contains operation if {
	some role, operations in role_allowed_operations
	some operation in operations
}

I ran some benchmarks (see rego files attached for full policy), and i'm seeing a noticeable changes in eval times

• p99 ~1000000ns to 500000ns
• p95 ~ 460000ns to to 32716ns

Steps To Reproduce

1. download the 2 rego files attached and rename from .txt to .rego
2. run ./opa bench --data issue_case_1.rego 'data.issue.operation_exists'
3. run ./opa bench --data issue_case_2.rego 'data.issue.operation_exists'
4. compare the eval time for each, the first use case should be much slower

issue_case_1.txt
issue_case_2.txt

Expected behavior

Performance should be equivalent in issue_case_1 & issue_case_2

Additional context

The iteration here is a fairly complex type of map<string, set<tuple<string, string>>>

[anderseknert]

Good find!

Looking at the output from the compiler (using opa-explorer), the nested iteration is left intact, while the some .. in loops are rewritten to something like this:

all_operations contains operation if {
    allowed_operation := role_allowed_operations[_]
    operation := allowed_operation[_]
}

Which seems to have the same negative impact compared to nesting the loop. I'd guess it's got something to do with all the intermediate values having to be realized on the "Rego side", but someone else (@johanfylling?) likely knows better :) If that is the case, it would be nice if the compiler could identify sequences of some .. in iteration and rewrite them to a single nested iteration where possible.

(btw, I think "more performant" should be "less performant" in the description 🙂)

[johanfylling]

I can confirm that evaluation of the "short-form" enumeration role_allowed_operations[_][operation] is slightly different, and it' wouldn't surprise me if this has a performance impact.
As @anderseknert suggests, I think it should be possible to detect this at compile-time (at least trivial cases), and optimize accordingly.

[stale]

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.