Skip to content

perf(normalizer): use Rc<str> interning to reduce alias resolution allocations #726

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anakrish wants to merge 1 commit into
base: main
Choose a base branch
from
+82 −33
Open

perf(normalizer): use Rc<str> interning to reduce alias resolution allocations #726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions src/languages/azure_policy/aliases/normalizer/alias_resolution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@
//! Per-alias path resolution: reads values from versioned ARM paths and places
//! them at alias short name paths in the normalized output.

use alloc::string::String;

use crate::Rc;
use crate::Value;

use super::super::obj_map::remove_element_field;
use super::super::obj_map::{
collision_safe_key, is_root_field_collision, obj_contains, obj_insert, obj_remove,
set_nested_lowercased, ObjMap,
collision_safe_key, is_root_field_collision, obj_contains, obj_insert, obj_insert_rc,
obj_remove, set_nested_lowercased, ObjMap,
};
use super::super::types::ResolvedAliases;
use super::element_remap::apply_element_remap_precomputed;
Expand Down Expand Up @@ -48,12 +47,14 @@ pub fn apply_alias_entries(
if let Some(value) = value {
let value = normalize_value(&value, &entry.short_name, None);

let target = if is_root_field_collision(&entry.short_name, &entry.default_path) {
collision_safe_key(&entry.short_name)
if is_root_field_collision(&entry.short_name, &entry.default_path) {
let target = collision_safe_key(&entry.short_name);
set_nested_lowercased(result, &target, value);
} else if entry.short_name.contains('.') {
set_nested_lowercased(result, &entry.short_name, value);
} else {
entry.short_name.clone()
};
set_nested_lowercased(result, &target, value);
obj_insert_rc(result, Rc::clone(&entry.short_name_lc), value);
}
}
}

Expand Down Expand Up @@ -84,13 +85,13 @@ pub fn apply_alias_entries(
}

/// Navigate an ARM path using precomputed segments (avoids per-call split).
fn navigate_arm_path_segments(value: &Value, segments: &[String]) -> Option<Value> {
fn navigate_arm_path_segments(value: &Value, segments: &[Rc<str>]) -> Option<Value> {
let mut current = value;
for segment in segments {
current = current
.as_object()
.ok()?
.get(&Value::from(segment.as_str()))?;
.get(&Value::String(Rc::clone(segment)))?;
}
Some(current.clone())
}
72 changes: 55 additions & 17 deletions src/languages/azure_policy/aliases/obj_map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//! then converts to `Value::Object` (a `BTreeMap<Value, Value>`) only at
//! the output boundary via [`make_value`].

use alloc::string::{String, ToString as _};
use alloc::string::String;
use alloc::vec::Vec;

use hashbrown::HashMap;
Expand Down Expand Up @@ -41,6 +41,33 @@ pub fn obj_insert(map: &mut ObjMap, key: &str, val: Value) {
map.insert(Rc::from(key), val);
}

/// Insert a key-value pair using a pre-allocated `Rc<str>` key.
///
/// Avoids the `Rc::from(key)` heap allocation that [`obj_insert`] performs.
pub fn obj_insert_rc(map: &mut ObjMap, key: Rc<str>, val: Value) {
map.insert(key, val);
}

/// Lowercase a string, returning an `Rc<str>`.
///
/// Both paths allocate an `Rc<str>` (header + string bytes). The fast-path
/// avoids creating an intermediate lowercased `String` when the input is
/// already all-lowercase ASCII.
pub fn rc_lowercase(s: &str) -> Rc<str> {
if s.bytes().all(|b| !b.is_ascii_uppercase()) {
Rc::from(s)
} else {
Rc::from(s.to_ascii_lowercase())
}
}

/// Insert a key-value pair with the key lowercased, using [`rc_lowercase`]
/// for the allocation fast-path.
pub fn obj_insert_lc(map: &mut ObjMap, key: &str, val: Value) {
let lc = rc_lowercase(key);
map.insert(lc, val);
}

/// Check whether a key exists.
pub fn obj_contains(map: &ObjMap, key: &str) -> bool {
map.contains_key(key)
Expand Down Expand Up @@ -112,7 +139,7 @@ pub fn set_nested_lowercased(result: &mut ObjMap, path: &str, value: Value) {
}
if segments.len() == 1 {
if let Some(&seg) = segments.first() {
obj_insert(result, &seg.to_ascii_lowercase(), value);
obj_insert_lc(result, seg, value);
}
return;
}
Expand Down Expand Up @@ -144,28 +171,28 @@ fn set_nested_inner(obj: &mut ObjMap, segments: &[&str], value: Value, lowercase
};

if segments.len() == 1 {
let key = if lowercase {
first.to_ascii_lowercase()
let key: Rc<str> = if lowercase {
rc_lowercase(first)
} else {
first.to_string()
Rc::from(first)
};
obj_insert(obj, &key, value);
obj_insert_rc(obj, key, value);
return;
}

let seg = if lowercase {
first.to_ascii_lowercase()
let seg: Rc<str> = if lowercase {
rc_lowercase(first)
} else {
first.to_string()
Rc::from(first)
};

// Ensure an intermediate object exists at `seg`.
if !obj_contains(obj, &seg) {
obj_insert(obj, &seg, make_value(new_map()));
if !obj.contains_key(&*seg) {
obj_insert_rc(obj, Rc::clone(&seg), make_value(new_map()));
}

// Descend directly into the BTreeMap, avoiding ObjMap round-trip.
if let Some(Value::Object(inner_rc)) = obj_get_mut(obj, &seg) {
if let Some(Value::Object(inner_rc)) = obj.get_mut(&*seg) {
let inner_btree = Rc::make_mut(inner_rc);
set_nested_in_btree(
inner_btree,
Expand All @@ -191,12 +218,12 @@ pub fn set_nested_in_btree(
return;
};

let key_str: String = if lowercase {
first.to_ascii_lowercase()
let key_rc: Rc<str> = if lowercase {
rc_lowercase(first)
} else {
first.to_string()
Rc::from(first)
};
let key_val = Value::String(Rc::from(key_str.as_str()));
let key_val = Value::String(Rc::clone(&key_rc));

if segments.len() == 1 {
btree.insert(key_val, value);
Expand Down Expand Up @@ -243,13 +270,24 @@ pub const ROOT_FIELDS: &[&str] = &[
"extendedLocation",
];

const PROPERTIES_DOT: &[u8] = b"properties.";

/// Check whether an alias short name collides with a reserved ARM root field
/// and needs a collision-safe key.
pub fn is_root_field_collision(short_name: &str, default_path: &str) -> bool {
ROOT_FIELDS
.iter()
.any(|f| f.eq_ignore_ascii_case(short_name))
&& default_path.to_ascii_lowercase().starts_with("properties.")
&& default_path.len() > PROPERTIES_DOT.len()
&& default_path
.as_bytes()
.get(..PROPERTIES_DOT.len())
.is_some_and(|prefix| {
prefix
.iter()
.zip(PROPERTIES_DOT)
.all(|(a, b)| a.to_ascii_lowercase() == *b)
})
}

/// Return a collision-safe key for an alias whose short name collides with a
Expand Down
20 changes: 15 additions & 5 deletions src/languages/azure_policy/aliases/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use alloc::vec::Vec;

use serde::{Deserialize, Deserializer};

use crate::Rc;

// ─── Top-level response wrappers ────────────────────────────────────────────

/// ARM API response envelope: `{ "value": [...] }`
Expand Down Expand Up @@ -404,11 +406,13 @@ pub struct ResolvedEntry {
// ── Precomputed fields (derived at registry-load time) ──────────────
/// Whether `short_name` contains `[*]` (i.e., this is a wildcard/array alias).
pub is_wildcard: bool,
/// Pre-lowercased short name as `Rc<str>` for allocation-free common-case inserts.
pub(crate) short_name_lc: Rc<str>,
/// Precomputed `default_path.split('.').collect()` for fast ARM path navigation.
pub default_path_segments: Vec<String>,
pub(crate) default_path_segments: Vec<Rc<str>>,
/// Precomputed path segments for each versioned path, in the same order
/// as `versioned_paths`.
pub versioned_path_segments: Vec<Vec<String>>,
pub(crate) versioned_path_segments: Vec<Vec<Rc<str>>>,
}

impl ResolvedEntry {
Expand All @@ -420,17 +424,23 @@ impl ResolvedEntry {
metadata: Option<AliasPathMetadata>,
) -> Self {
let is_wildcard = short_name.contains("[*]");
let default_path_segments = default_path.split('.').map(String::from).collect();
let short_name_lc = if short_name.bytes().all(|b| !b.is_ascii_uppercase()) {
Rc::from(short_name.as_str())
} else {
Rc::from(short_name.to_ascii_lowercase())
};
let default_path_segments = default_path.split('.').map(Rc::from).collect();
let versioned_path_segments = versioned_paths
.iter()
.map(|(_, p)| p.split('.').map(String::from).collect())
.map(|(_, p)| p.split('.').map(Rc::from).collect())
.collect();
Self {
short_name,
default_path,
versioned_paths,
metadata,
is_wildcard,
short_name_lc,
default_path_segments,
versioned_path_segments,
}
Expand All @@ -456,7 +466,7 @@ impl ResolvedEntry {
/// Returns the versioned segments if `api_version` matches, otherwise
/// the default segments. This avoids per-call `split('.')` for both
/// default and versioned scalar alias navigation.
pub fn select_path_segments(&self, api_version: Option<&str>) -> &[String] {
pub(crate) fn select_path_segments(&self, api_version: Option<&str>) -> &[Rc<str>] {
if let Some(ver) = api_version {
for (i, (v, _)) in self.versioned_paths.iter().enumerate() {
if v.eq_ignore_ascii_case(ver) {
Expand Down
Loading