fix(authz): optimize subject mapping evaluation performance #2945
Conversation
Add performance optimizations to subject mapping evaluation hot paths: - Flattening: Add internal index map for O(1) selector lookups - GetFromFlattened_Large: 39x faster (217ns → 5.5ns), zero allocs - GetFromFlattened_MultipleLookups: 17x faster, zero allocs - EvaluateCondition: Set-based IN/NOT_IN with early returns - EvaluateCondition_IN_Large: 2.3x faster - EvaluateCondition_IN_Hit: 4.6x faster, zero allocs - Per-entity caches for SubjectSet/ConditionGroup evaluation - RepeatedSubjectSets: 21% faster, 37% fewer allocs - Action deduplication using per-valueFQN accumulator - WithActions: 6% faster, 51% fewer allocations - HierarchyRule: O(h) definition scan vs O(e) entitlements scan Trade-off: Flatten() is ~2x slower due to index building, but this cost is amortized over many GetFromFlattened calls per entity.
Summary of ChangesHello @strantalis, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a suite of performance optimizations across the authorization subject mapping evaluation logic. The primary goal is to significantly reduce the computational overhead and improve the response times of critical authorization paths. This is achieved through strategic use of data structures for faster lookups, caching mechanisms to avoid redundant calculations, and algorithmic improvements in core evaluation functions. The changes aim to make the authorization system more scalable and efficient, particularly under heavy load or with complex policy definitions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Code runs slow, then fast, Maps and caches, quick to find, Authz now takes flight. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a series of significant performance optimizations to the authorization and subject mapping evaluation paths. The changes are well-structured and demonstrate a deep understanding of the performance characteristics of the code.
Key improvements include:
- O(1) lookups in
flattening: By pre-building an index,GetFromFlattenedis now significantly faster, with benchmarks showing zero allocations on cache hits. - Efficient condition evaluation: The
INandNOT_INoperators inEvaluateConditionnow use hash sets, reducing the complexity from O(N*M) to O(N+M). - Caching: Per-entity caching for
SubjectSetandConditionGroupevaluations avoids redundant computations for repeated policy structures. - Correct action deduplication: The logic in
EvaluateSubjectMappingsWithActionsnow correctly deduplicates actions across multiple subject mappings for the same FQN, which is both a performance win and a correctness fix. - Optimized hierarchy rule: The
hierarchyRulenow scans a much smaller set of definitions, improving its efficiency.
The addition of comprehensive benchmarks is excellent, as it validates the performance gains claimed.
I have one suggestion to improve consistency in the flattening package. Overall, this is an excellent contribution.
Fixed bug where subjectPropertySet was checked but never updated, causing duplicate selectors to be added.
strantalis
force-pushed
the
fix/sm-perf-improvements
branch
3 times, most recently
from
2dcf1c9 to
8c10f4e
Compare
Add new benchmark commands that exercise subject mapping evaluation with multiple attributes per resource and multiple resources per request: - benchmark-decision-complex: v2 API with configurable resources/attrs - benchmark-decision-v1-complex: v1 API with same capabilities These benchmarks show where optimization improvements shine by testing with 500 resources × 5 attributes = 2,500 attribute checks per request. Also updates CI workflow to run and report these benchmarks.
strantalis
force-pushed
the
fix/sm-perf-improvements
branch
from
8c10f4e to
c5ec090
Compare
The complex benchmarks were not providing useful metrics as all decisions were being denied. Removing them to simplify the CI.
- Flatten: separate index building into a linear pass, skip index for structures with fewer than 8 items (linear scan is faster) - EvaluateSubjectSet/ConditionGroup: split into cached and non-cached versions, use non-cached for single evaluations to avoid map overhead - IN/NOT_IN conditions: use linear search for small value sets (<=4) to avoid map allocation Benchmarks show -38% geomean improvement in subject mapping evaluation and -66% geomean improvement in flattening lookups.
There was a problem hiding this comment.
Pull request overview
This PR optimizes subject mapping evaluation performance through targeted improvements in hot paths. The optimizations include flattening index maps for O(1) lookups, set-based IN/NOT_IN evaluation with early returns, per-entity caching for SubjectSet/ConditionGroup evaluations, streamlined action deduplication, and a more efficient hierarchy rule scan. The changes maintain backward compatibility while delivering significant performance gains (up to 97% faster for large dataset lookups) with reduced memory allocations.
Key Changes
- Flattening optimization with internal index map provides O(1) selector lookups (39x faster for large datasets)
- Set-based IN/NOT_IN operators with threshold-based algorithm selection (2-5x faster)
- Per-entity caching for repeated SubjectSet/ConditionGroup evaluations (21-47% faster, 37-57% fewer allocations)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
lib/flattening/flatten.go |
Adds internal index map for O(1) selector lookups when structure size exceeds threshold (8 items) |
lib/flattening/flatten_bench_large_test.go |
New benchmarks demonstrating flattening lookup performance improvements |
service/internal/subjectmappingbuiltin/subject_mapping_builtin.go |
Implements per-entity caching, optimized IN/NOT_IN operators with threshold-based algorithm selection, and fixes spelling in comments |
service/internal/subjectmappingbuiltin/subject_mapping_builtin_actions.go |
Refactors action deduplication to use per-valueFQN accumulator pattern, reducing allocations |
service/internal/subjectmappingbuiltin/subject_mapping_bench_large_test.go |
Comprehensive benchmarks covering large attribute counts, repeated subject sets, and multiple entities |
service/internal/access/v2/evaluate.go |
Optimizes hierarchy rule to scan O(h) definition entries instead of O(e) entitlements |
service/internal/access/v2/just_in_time_pdp.go |
Fixes bug where subject properties could be duplicated by properly tracking added keys |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a series of significant performance optimizations to the subject mapping evaluation paths, backed by comprehensive benchmarks demonstrating impressive gains. The changes include introducing an index for O(1) lookups in flattened structures, using set-based logic for IN/NOT_IN operations, caching evaluation results, and improving action deduplication logic. The code is well-structured and the optimizations are clearly beneficial.
My review includes a couple of suggestions to improve robustness: one to maintain backward compatibility in a modified library function's return value, and another to ensure deterministic output by sorting results from map iteration. Overall, this is an excellent set of performance improvements.
service/internal/subjectmappingbuiltin/subject_mapping_builtin_actions.go
Show resolved
Hide resolved
| flattened.Items = items | ||
| return flattened, nil | ||
|
|
||
| // Build index in a separate pass, only for larger structures |
There was a problem hiding this comment.
It seems like we could improve this further by updating flattenInterface to build the index lookup as well so the passes drop from 2n to n.
Add TestHierarchyRule_DefinitionMissingValues to exercise the defensive code path that handles data inconsistency where resourceValueFQNs contains values not present in attrDefinition.GetValues(). This guards against panics if the policy service returns incomplete attribute definitions.
strantalis
changed the title
2 participants
Proposed Changes
Add performance optimizations to subject mapping evaluation hot paths:
Flattening: O(1) selector lookups via index map
EvaluateCondition: Optimized IN/NOT_IN evaluation
Per-entity caches for SubjectSet/ConditionGroup evaluation
Action deduplication using per-valueFQN accumulator
Overall impact
Trade-offs
sm-old.txt
flatten-old.txt
flatten-new.txt
sm-new.txt
Checklist
Testing Instructions