refactor(azure_policy): reduce AliasRegistry allocations via Rc sharing#725
Open
anakrish wants to merge 1 commit into
Open
refactor(azure_policy): reduce AliasRegistry allocations via Rc sharing#725anakrish wants to merge 1 commit into
anakrish wants to merge 1 commit into
Conversation
538f8a9 to
8a322ff
Compare
There was a problem hiding this comment.
Pull request overview
Optimizes the Azure Policy AliasRegistry to reduce allocations on the load and compile paths, and hardens JSON deserialization against null arrays emitted by az provider list. The compiler now stores Rc<BTreeMap<...>> for alias data so repeated compilations share a single 73K-entry map instead of cloning, and ingest_alias_entries works on borrowed &str slices for short-name derivation.
Changes:
alias_map()/alias_modifiable_map()return cachedRc<BTreeMap>; compiler holds theRcinstead of an owned clone.ingest_alias_entriesderives short names via borrowed slices and safe.get()-based indexing, reducing per-alias allocations.- Adds
deserialize_null_as_empty_vecforresourceTypes/aliases/paths/apiVersions, and replaces an#[allow(clippy::indexing_slicing)]block inbuild_object_from_keyswith safe lookups.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/languages/azure_policy/aliases/mod.rs | Adds Rc cache for alias maps, refactors short-name derivation, adds lowercase-key lookup helpers. |
| src/languages/azure_policy/aliases/types.rs | Adds deserialize_null_as_empty_vec and applies it to four Vec fields. |
| src/languages/azure_policy/compiler/core.rs | Compiler fields switched from BTreeMap to Rc<BTreeMap>. |
| src/languages/azure_policy/compiler/mod.rs | compile_*_with_aliases signatures updated to take Rc<BTreeMap>. |
| src/languages/azure_policy/compiler/effects.rs | Replaces unchecked indexing in build_object_from_keys with safe .get() + error propagation. |
| examples/regorus/azure_policy.rs | Adjusts example to bind the returned Rc map before iterating. |
| tests/azure_policy/mod.rs | Makes alias_registry mutable to satisfy new &mut self accessors. |
Collaborator
Author
|
@copilot review this PR from different perspectives not considered before. |
cf77d19 to
6f2fd04
Compare
6f2fd04 to
70b2b38
Compare
91aa927 to
fdae5db
Compare
Replace the compiler's two cloned BTreeMap fields (alias_map and alias_modifiable) with a single Option<Rc<AliasRegistry>>. This eliminates cloning two 73K-entry maps on every compilation by sharing the registry through a reference-counted pointer. Additional improvements: - alias_map()/alias_modifiable_map() return &BTreeMap (zero-copy) - Safe .get() indexing in ingest_alias_entries and build_object_from_keys - Null-tolerant deserialization for AliasEntry.paths (~97% of real Azure catalog entries emit "paths": null) All changes are within the azure_policy feature gate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fdae5db to
0c674e7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Eliminates cloning two 73K-entry
BTreeMaps on every Azure Policy compilation by sharing theAliasRegistrythrough a reference-counted pointer.Changes
Compiler — replaces two owned map fields (
alias_map,alias_modifiable) withOption<Rc<AliasRegistry>>. The threecompile_*_with_aliasesentry points now takeRc<AliasRegistry>instead of two separate maps:AliasRegistry —
alias_map()andalias_modifiable_map()now return&BTreeMapreferences instead of clones.Safe indexing — replaces direct
[..]slicing with.get()+ error propagation iningest_alias_entriesandbuild_object_from_keys, satisfying the crate-widedeny(clippy::indexing_slicing)lint without#[allow]overrides.Null-tolerant deserialization — custom deserializer for
AliasEntry.pathshandles the ~97% of real Azure catalog entries whereaz provider listemits"paths": null.Scope
All changes are within the
azure_policyfeature gate. No impact on the core engine, FFI bindings, or other language backends.