Skip to content

Commit 91aa927

Browse files
anakrishCopilot
andcommitted
refactor(azure_policy): reduce AliasRegistry allocations via Rc sharing
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>
1 parent d2c483e commit 91aa927

8 files changed

Lines changed: 107 additions & 72 deletions

File tree

examples/regorus/azure_policy.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ use regorus::languages::azure_policy::aliases::AliasRegistry;
2323
use regorus::languages::azure_policy::compiler;
2424
use regorus::languages::azure_policy::parser;
2525
use regorus::rvm::RegoVM;
26-
use regorus::Source;
27-
use regorus::Value;
26+
use regorus::{Rc, Source, Value};
2827

2928
/// Evaluate an Azure Policy definition against a resource.
3029
///
@@ -60,11 +59,8 @@ pub fn azure_policy_eval(
6059
println!("Parsed policy definition from {policy_definition}");
6160

6261
// 3. Compile to RVM bytecode.
63-
let program = compiler::compile_policy_definition_with_aliases(
64-
&defn,
65-
registry.alias_map(),
66-
registry.alias_modifiable_map(),
67-
)?;
62+
let registry = Rc::new(registry);
63+
let program = compiler::compile_policy_definition_with_aliases(&defn, Rc::clone(&registry))?;
6864
println!("Compiled policy to RVM bytecode");
6965

7066
// 4. Build normalized input.
@@ -138,7 +134,7 @@ pub fn azure_policy_aliases(aliases: String, resource_type: Option<String>) -> R
138134
if let Some(ref rt) = resource_type {
139135
let rt_lower = rt.to_lowercase();
140136
let mut found = false;
141-
for (alias_name, _) in registry.alias_map() {
137+
for alias_name in registry.alias_map().keys() {
142138
if alias_name.to_lowercase().starts_with(&rt_lower) {
143139
println!(" {alias_name}");
144140
found = true;
@@ -148,7 +144,7 @@ pub fn azure_policy_aliases(aliases: String, resource_type: Option<String>) -> R
148144
bail!("no aliases found for resource type '{rt}'");
149145
}
150146
} else {
151-
for (alias_name, _) in registry.alias_map() {
147+
for alias_name in registry.alias_map().keys() {
152148
println!(" {alias_name}");
153149
}
154150
}

src/languages/azure_policy/aliases/mod.rs

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,27 @@ impl AliasRegistry {
172172
let prefix = alloc::format!("{}/", fq_type);
173173

174174
for alias in aliases {
175+
// Skip aliases with no usable path — the normalizer also skips
176+
// these (resolve_resource_type checks default_path), so inserting
177+
// them into the compiler maps would cause a divergence where the
178+
// compiler resolves the alias but the normalized input never
179+
// contains the corresponding field.
180+
if alias.default_path.is_none() && alias.paths.is_empty() {
181+
continue;
182+
}
183+
175184
// Derive the short name by stripping the resource type prefix.
176185
let raw_short = if alias.name.len() > prefix.len()
177-
&& alias.name[..prefix.len()].eq_ignore_ascii_case(&prefix)
186+
&& alias
187+
.name
188+
.get(..prefix.len())
189+
.is_some_and(|s| s.eq_ignore_ascii_case(&prefix))
178190
{
179-
alias.name[prefix.len()..].to_string()
191+
alias
192+
.name
193+
.get(prefix.len()..)
194+
.unwrap_or(&alias.name)
195+
.to_string()
180196
} else if let Some(rest) = alias
181197
.name
182198
.rfind('/')
@@ -260,20 +276,19 @@ impl AliasRegistry {
260276
.map(String::as_str)
261277
}
262278

263-
/// Return a clone of the alias-to-short-name map for use by the compiler.
279+
/// Return a reference to the alias-to-short-name map.
264280
///
265-
/// The compiler stores this map internally so it can resolve fully-qualified
266-
/// alias names without holding a reference to the registry.
267-
pub fn alias_map(&self) -> BTreeMap<String, String> {
268-
self.alias_to_short.clone()
281+
/// Keys are lowercase fully-qualified alias names; values are short names.
282+
pub const fn alias_map(&self) -> &BTreeMap<String, String> {
283+
&self.alias_to_short
269284
}
270285

271-
/// Return a clone of the alias-to-modifiable map for use by the compiler.
286+
/// Return a reference to the alias-to-modifiable map.
272287
///
273-
/// Maps lowercase fully-qualified alias names to `true` when the alias
274-
/// has `defaultMetadata.attributes = "Modifiable"`.
275-
pub fn alias_modifiable_map(&self) -> BTreeMap<String, bool> {
276-
self.alias_modifiable.clone()
288+
/// Keys are lowercase fully-qualified alias names; values are `true` when
289+
/// the alias has `defaultMetadata.attributes = "Modifiable"`.
290+
pub const fn alias_modifiable_map(&self) -> &BTreeMap<String, bool> {
291+
&self.alias_modifiable
277292
}
278293

279294
/// Normalize a raw ARM resource and wrap it in the input envelope.

src/languages/azure_policy/aliases/types.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,20 @@ use alloc::vec::Vec;
1818

1919
use serde::{Deserialize, Deserializer};
2020

21+
// ---------------------------------------------------------------------------
22+
// Deserialization helpers
23+
// ---------------------------------------------------------------------------
24+
25+
/// Deserialize a `Vec<T>` that tolerates JSON `null` by mapping it to an
26+
/// empty vector.
27+
fn deserialize_null_as_empty_vec<'de, T, D>(deserializer: D) -> Result<Vec<T>, D::Error>
28+
where
29+
T: Deserialize<'de>,
30+
D: Deserializer<'de>,
31+
{
32+
Ok(Option::<Vec<T>>::deserialize(deserializer)?.unwrap_or_default())
33+
}
34+
2135
// ─── Top-level response wrappers ────────────────────────────────────────────
2236

2337
/// ARM API response envelope: `{ "value": [...] }`
@@ -98,7 +112,10 @@ pub struct AliasEntry {
98112

99113
/// Versioned path entries. Empty for the vast majority of aliases that
100114
/// have only a `defaultPath`.
101-
#[serde(default)]
115+
///
116+
/// In real Azure catalog data (~97% of aliases), `az provider list` emits
117+
/// `"paths": null` rather than an empty array.
118+
#[serde(default, deserialize_with = "deserialize_null_as_empty_vec")]
102119
pub paths: Vec<AliasPath>,
103120
}
104121

src/languages/azure_policy/compiler/core.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use crate::rvm::program::{Program, SpanInfo};
1818
use crate::rvm::Instruction;
1919
use crate::{Rc, Value};
2020

21+
use crate::languages::azure_policy::aliases::AliasRegistry;
2122
use crate::languages::azure_policy::ast::PolicyRule;
2223

2324
// ---------------------------------------------------------------------------
@@ -44,10 +45,9 @@ pub(super) struct Compiler {
4445
pub(super) cached_input_reg: Option<u8>,
4546
/// Cached register for `LoadContext` — allocated once on first use.
4647
pub(super) cached_context_reg: Option<u8>,
47-
/// Map from lowercase fully-qualified alias name → short name.
48-
pub(super) alias_map: BTreeMap<String, String>,
49-
/// Map from lowercase fully-qualified alias name → modifiable flag.
50-
pub(super) alias_modifiable: BTreeMap<String, bool>,
48+
/// Alias registry for resolving fully-qualified alias names.
49+
/// Shared via `Rc` to avoid cloning the 73K-entry alias maps.
50+
pub(super) alias_registry: Option<Rc<AliasRegistry>>,
5151
/// Default values for policy parameters.
5252
pub(super) parameter_defaults: Option<Value>,
5353
/// Cached literal-table index for `parameter_defaults` (or an empty object
@@ -338,8 +338,13 @@ impl Compiler {
338338
path: &str,
339339
span: &crate::lexer::Span,
340340
) -> Result<String> {
341+
let alias_map = match &self.alias_registry {
342+
Some(reg) => reg.alias_map(),
343+
None => return Ok(path.to_string()),
344+
};
345+
341346
let lc = path.to_ascii_lowercase();
342-
if let Some(short) = self.alias_map.get(&lc) {
347+
if let Some(short) = alias_map.get(&lc) {
343348
let resolved = short.clone();
344349
let result = Self::strip_fq_prefix(&resolved).to_ascii_lowercase();
345350
return Ok(result);
@@ -348,22 +353,22 @@ impl Compiler {
348353
// Fallback: derive array path from a corresponding `[*]` alias.
349354
if !lc.contains("[*]") {
350355
let wildcard_key = alloc::format!("{}[*]", lc);
351-
if let Some(short) = self.alias_map.get(&wildcard_key) {
356+
if let Some(short) = alias_map.get(&wildcard_key) {
352357
let resolved = Self::strip_fq_prefix(short).to_ascii_lowercase();
353358
if let Some(base) = resolved.strip_suffix("[*]") {
354359
return Ok(base.to_string());
355360
}
356361
}
357362
}
358363

359-
if !self.alias_map.is_empty() && !self.alias_fallback_to_raw {
364+
if !alias_map.is_empty() && !self.alias_fallback_to_raw {
360365
bail!(span.error(&alloc::format!(
361366
"unknown alias '{}': field references must use fully-qualified alias names when an alias catalog is loaded",
362367
path
363368
)));
364369
}
365370

366-
if self.alias_map.is_empty() {
371+
if alias_map.is_empty() {
367372
Ok(path.to_string())
368373
} else {
369374
let result = Self::strip_fq_prefix(path).to_ascii_lowercase();

src/languages/azure_policy/compiler/count.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -998,7 +998,13 @@ impl Compiler {
998998
return Ok(result);
999999
}
10001000
}
1001-
Err(e) if !self.alias_map.is_empty() && !self.alias_fallback_to_raw => {
1001+
Err(e)
1002+
if self
1003+
.alias_registry
1004+
.as_ref()
1005+
.is_some_and(|r| !r.alias_map().is_empty())
1006+
&& !self.alias_fallback_to_raw =>
1007+
{
10021008
return Err(e);
10031009
}
10041010
_ => {}

src/languages/azure_policy/compiler/effects.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -692,13 +692,18 @@ impl Compiler {
692692
field_path: &str,
693693
span: &crate::lexer::Span,
694694
) -> Result<()> {
695-
if self.alias_modifiable.is_empty() {
695+
let modifiable_map = match &self.alias_registry {
696+
Some(reg) => reg.alias_modifiable_map(),
697+
None => return Ok(()),
698+
};
699+
700+
if modifiable_map.is_empty() {
696701
return Ok(());
697702
}
698703

699704
let lc = field_path.to_lowercase();
700705

701-
if let Some(&modifiable) = self.alias_modifiable.get(&lc) {
706+
if let Some(&modifiable) = modifiable_map.get(&lc) {
702707
if !modifiable {
703708
bail!(span.error(&format!(
704709
"alias '{}' is not modifiable (defaultMetadata.attributes != 'Modifiable')",
@@ -803,7 +808,6 @@ fn unescape_arm_literal(s: &str) -> alloc::string::String {
803808
/// 1. Build a template `BTreeMap` with `Value::Undefined` placeholders.
804809
/// 2. Sort keys by their literal value (BTreeMap order).
805810
/// 3. Emit `ObjectCreate`.
806-
#[allow(clippy::indexing_slicing)]
807811
pub(super) fn build_object_from_keys(
808812
compiler: &mut Compiler,
809813
mut keys: Vec<(u16, u8)>,
@@ -812,17 +816,21 @@ pub(super) fn build_object_from_keys(
812816
// Build template: object with all keys set to Undefined.
813817
let mut template = BTreeMap::new();
814818
for &(key_idx, _) in &keys {
815-
// SAFETY: key_idx was just returned by `add_literal_u16`, so the
816-
// index is guaranteed to be in bounds.
817-
let key_val = compiler.program.literals[usize::from(key_idx)].clone();
819+
let key_val = compiler
820+
.program
821+
.literals
822+
.get(usize::from(key_idx))
823+
.ok_or_else(|| anyhow!("literal index out of bounds"))?
824+
.clone();
818825
template.insert(key_val, Value::Undefined);
819826
}
820827
let template_idx = compiler.add_literal_u16(Value::Object(crate::Rc::new(template)))?;
821828

822829
// Sort keys by literal value (BTreeMap order).
823830
keys.sort_by(|a, b| {
824-
compiler.program.literals[usize::from(a.0)]
825-
.cmp(&compiler.program.literals[usize::from(b.0)])
831+
let a_val = compiler.program.literals.get(usize::from(a.0));
832+
let b_val = compiler.program.literals.get(usize::from(b.0));
833+
a_val.cmp(&b_val)
826834
});
827835

828836
let dest = compiler.alloc_register()?;

src/languages/azure_policy/compiler/mod.rs

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ mod metadata;
3030
mod template_dispatch;
3131
mod utils;
3232

33-
use alloc::collections::BTreeMap;
34-
use alloc::string::{String, ToString as _};
33+
use alloc::string::ToString as _;
3534

3635
use anyhow::Result;
3736

37+
use crate::languages::azure_policy::aliases::AliasRegistry;
3838
use crate::languages::azure_policy::ast::{PolicyDefinition, PolicyRule};
3939
use crate::rvm::program::Program;
4040
use crate::{Rc, Value};
@@ -69,17 +69,14 @@ pub fn compile_policy_rule(rule: &PolicyRule) -> Result<Rc<Program>> {
6969

7070
/// Compile a parsed Azure Policy rule with alias resolution.
7171
///
72-
/// The `alias_map` maps lowercase fully-qualified alias names to their short
73-
/// names. Obtain it from
74-
/// [`AliasRegistry::alias_map()`](crate::languages::azure_policy::aliases::AliasRegistry::alias_map).
72+
/// The registry provides alias-to-short-name resolution and modifiability
73+
/// data. Pass it as an `Rc` to avoid cloning the internal alias maps.
7574
pub fn compile_policy_rule_with_aliases(
7675
rule: &PolicyRule,
77-
alias_map: BTreeMap<String, String>,
78-
alias_modifiable: BTreeMap<String, bool>,
76+
registry: Rc<AliasRegistry>,
7977
) -> Result<Rc<Program>> {
8078
let mut compiler = Compiler::new();
81-
compiler.alias_map = alias_map;
82-
compiler.alias_modifiable = alias_modifiable;
79+
compiler.alias_registry = Some(registry);
8380
init_effect_annotation(&mut compiler, rule);
8481
compiler.compile(rule)
8582
}
@@ -100,12 +97,10 @@ pub fn compile_policy_definition(defn: &PolicyDefinition) -> Result<Rc<Program>>
10097
/// Compile a parsed Azure Policy definition with alias resolution.
10198
pub fn compile_policy_definition_with_aliases(
10299
defn: &PolicyDefinition,
103-
alias_map: BTreeMap<String, String>,
104-
alias_modifiable: BTreeMap<String, bool>,
100+
registry: Rc<AliasRegistry>,
105101
) -> Result<Rc<Program>> {
106102
let mut compiler = Compiler::new();
107-
compiler.alias_map = alias_map;
108-
compiler.alias_modifiable = alias_modifiable;
103+
compiler.alias_registry = Some(registry);
109104
compiler.parameter_defaults = Some(build_parameter_defaults(&defn.parameters)?);
110105
compiler.populate_definition_metadata(defn);
111106
init_effect_annotation(&mut compiler, &defn.policy_rule);
@@ -119,13 +114,11 @@ pub fn compile_policy_definition_with_aliases(
119114
/// a known alias are silently treated as raw property paths.
120115
pub fn compile_policy_definition_with_aliases_opts(
121116
defn: &PolicyDefinition,
122-
alias_map: BTreeMap<String, String>,
123-
alias_modifiable: BTreeMap<String, bool>,
117+
registry: Rc<AliasRegistry>,
124118
alias_fallback_to_raw: bool,
125119
) -> Result<Rc<Program>> {
126120
let mut compiler = Compiler::new();
127-
compiler.alias_map = alias_map;
128-
compiler.alias_modifiable = alias_modifiable;
121+
compiler.alias_registry = Some(registry);
129122
compiler.alias_fallback_to_raw = alias_fallback_to_raw;
130123
compiler.parameter_defaults = Some(build_parameter_defaults(&defn.parameters)?);
131124
compiler.populate_definition_metadata(defn);

0 commit comments

Comments
 (0)