diff --git a/crates/superposition_core/benches/resolve.rs b/crates/superposition_core/benches/resolve.rs index 8a53c595a..55b6de89f 100644 --- a/crates/superposition_core/benches/resolve.rs +++ b/crates/superposition_core/benches/resolve.rs @@ -7,12 +7,12 @@ //! operation the OpenFeature provider performs on every flag evaluation via //! `superposition_core::eval_config`. //! -//! Two variants are compared: -//! * `optimized_borrowed` — the current implementation, which resolves -//! directly against the borrowed context/override set. -//! * `pre_optimization_cloned` — reproduces the pre-optimization overhead, -//! where every resolve deep-cloned the entire context set (once to read -//! the dimensions, once more to build a throwaway `Config`). +//! Three variants are compared: +//! * `optimized_borrowed` — resolves all config without prefix filtering. +//! * `prefix_pre_optimization_cloned` — reproduces the old prefix path, +//! which cloned and filtered the complete config before resolving. +//! * `prefix_optimized_borrowed` — resolves a prefix-filtered config while +//! borrowing the context/override set. //! //! Run with: //! cargo bench -p superposition_core --bench resolve @@ -22,15 +22,15 @@ //! BENCH_CONTEXTS=50000 cargo bench -p superposition_core --bench resolve use std::cell::Cell; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::time::{Duration, Instant}; -use criterion::{Criterion, black_box, criterion_group, criterion_main}; -use serde_json::{Map, Value, json}; -use superposition_core::{Config, MergeStrategy, eval_config}; +use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use serde_json::{json, Map, Value}; +use superposition_core::{eval_config, Config, MergeStrategy}; use superposition_types::{ - Cac, Condition, Context, DimensionInfo, ExtendedMap, OverrideWithKeys, Overrides, database::models::cac::{DependencyGraph, DimensionType}, + Cac, Condition, Context, DimensionInfo, ExtendedMap, OverrideWithKeys, Overrides, }; /// Number of regular dimensions in the synthetic dataset (mirrors the reported data). @@ -43,6 +43,7 @@ const NUM_QUERIES: usize = 100; const DIMENSION_CARDINALITY: usize = 12; /// The (small) set of default config keys. const DEFAULT_CONFIG_KEYS: [&str; 3] = ["config.alpha", "config.beta", "config.gamma"]; +const PREFIX_FILTER: [&str; 1] = ["config.a"]; /// Tiny deterministic xorshift64 PRNG so the dataset is fully reproducible /// without pulling in an external `rand` dependency. @@ -235,8 +236,10 @@ fn build_dataset(num_contexts: usize) -> Dataset { } } -/// Current implementation: resolve directly against the borrowed data. -fn resolve_optimized(ds: &Dataset, query: &Map) -> Map { +fn resolve_prefix_optimized( + ds: &Dataset, + query: &Map, +) -> Map { eval_config( ds.default_config.clone(), &ds.contexts, @@ -244,36 +247,56 @@ fn resolve_optimized(ds: &Dataset, query: &Map) -> Map, ) -> Map { - // (1) Old `get_dimensions_info`: cloned the whole Config just to read - // `.dimensions`. - let full = Config { + let prefixes = PREFIX_FILTER + .iter() + .map(|prefix| prefix.to_string()) + .collect::>(); + + let Config { + contexts, + overrides, + default_configs, + dimensions, + } = Config { contexts: ds.contexts.clone(), overrides: ds.overrides.clone(), default_configs: ds.default_config.clone().into(), dimensions: ds.dimensions.clone(), - }; - let _dimensions = full.dimensions.clone(); + } + .filter_by_prefix(&prefixes); - // (2) Old `eval_config`: cloned contexts + overrides into a temporary - // Config before resolving. - let contexts = ds.contexts.clone(); - let overrides = ds.overrides.clone(); eval_config( - ds.default_config.clone(), + default_configs.into_inner(), &contexts, &overrides, + &dimensions, + query, + MergeStrategy::MERGE, + None, + ) + .expect("resolve") +} + +/// Current implementation: resolve directly against the borrowed data. +fn resolve_optimized(ds: &Dataset, query: &Map) -> Map { + eval_config( + ds.default_config.clone(), + &ds.contexts, + &ds.overrides, &ds.dimensions, query, MergeStrategy::MERGE, @@ -314,12 +337,21 @@ fn bench_resolve(c: &mut Criterion) { }) }); - group.bench_function("pre_optimization_cloned", |b| { + group.bench_function("prefix_optimized_borrowed", |b| { + let counter = Cell::new(0usize); + b.iter(|| { + let q = &ds.queries[counter.get() % ds.queries.len()]; + counter.set(counter.get() + 1); + black_box(resolve_prefix_optimized(&ds, q)) + }) + }); + + group.bench_function("prefix_pre_optimization_cloned", |b| { let counter = Cell::new(0usize); b.iter(|| { let q = &ds.queries[counter.get() % ds.queries.len()]; counter.set(counter.get() + 1); - black_box(resolve_pre_optimization(&ds, q)) + black_box(resolve_prefix_pre_optimization(&ds, q)) }) }); diff --git a/crates/superposition_core/src/config.rs b/crates/superposition_core/src/config.rs index c7281c963..e7932a52b 100644 --- a/crates/superposition_core/src/config.rs +++ b/crates/superposition_core/src/config.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use serde_json::{Map, Value}; pub use superposition_types::api::config::MergeStrategy; use superposition_types::{ - logic::evaluate_local_cohorts, Config, Context, DimensionInfo, Overrides, + logic::evaluate_local_cohorts, Context, DimensionInfo, Overrides, }; pub fn eval_config( @@ -19,51 +19,26 @@ pub fn eval_config( // leaves untouched, so it is safe to compute once regardless of the path. let modified_query_data = evaluate_local_cohorts(dimensions, query_data); - let filter_prefixes = filter_prefixes.filter(|p| !p.is_empty()); - - // Fast path: no prefix filtering. Resolve directly against the borrowed - // contexts/overrides instead of deep-cloning the entire context set (which - // can be hundreds of thousands of entries) into a temporary `Config`. - let Some(prefixes) = filter_prefixes else { - let overrides_map = get_overrides( - &modified_query_data, - contexts, - overrides, - &merge_strategy, - None, - )?; - - let mut result_config = default_config; - merge_overrides_on_default_config( - &mut result_config, - overrides_map, - &merge_strategy, - ); - return Ok(result_config); - }; - - // Slow path: prefix filtering needs an owned, filtered `Config`. - let config = Config { - default_configs: default_config.into(), - contexts: contexts.to_vec(), - overrides: overrides.clone(), - dimensions: dimensions.clone(), - } - .filter_by_prefix(&HashSet::from_iter(prefixes)); + let filter_prefixes: Option> = filter_prefixes + .filter(|prefixes| !prefixes.is_empty()) + .map(HashSet::from_iter); let overrides_map: Map = get_overrides( &modified_query_data, - &config.contexts, - &config.overrides, + contexts, + overrides, &merge_strategy, + filter_prefixes.as_ref(), None, )?; - // Apply overrides to default config - let mut result_config = config.default_configs; + let mut result_config = match &filter_prefixes { + Some(prefixes) => filter_config_keys_by_prefix(default_config, prefixes), + None => default_config, + }; merge_overrides_on_default_config(&mut result_config, overrides_map, &merge_strategy); - Ok(result_config.into_inner()) + Ok(result_config) } pub fn eval_config_with_reasoning( @@ -77,46 +52,26 @@ pub fn eval_config_with_reasoning( ) -> Result, String> { let modified_query_data = evaluate_local_cohorts(dimensions, query_data); - let filter_prefixes = filter_prefixes.filter(|p| !p.is_empty()); - - let Some(prefixes) = filter_prefixes else { - let overrides_map = get_overrides( - &modified_query_data, - contexts, - overrides, - &merge_strategy, - None, - )?; - - let mut result_config = default_config; - merge_overrides_on_default_config( - &mut result_config, - overrides_map, - &merge_strategy, - ); - return Ok(result_config); - }; - - let config = Config { - default_configs: default_config.into(), - contexts: contexts.to_vec(), - overrides: overrides.clone(), - dimensions: dimensions.clone(), - } - .filter_by_prefix(&HashSet::from_iter(prefixes)); + let filter_prefixes: Option> = filter_prefixes + .filter(|prefixes| !prefixes.is_empty()) + .map(HashSet::from_iter); let overrides_map = get_overrides( &modified_query_data, - &config.contexts, - &config.overrides, + contexts, + overrides, &merge_strategy, + filter_prefixes.as_ref(), None, )?; - let mut result_config = config.default_configs; + let mut result_config = match &filter_prefixes { + Some(prefixes) => filter_config_keys_by_prefix(default_config, prefixes), + None => default_config, + }; merge_overrides_on_default_config(&mut result_config, overrides_map, &merge_strategy); - Ok(result_config.into_inner()) + Ok(result_config) } pub fn merge(doc: &mut Value, patch: &Value) { @@ -135,11 +90,28 @@ pub fn merge(doc: &mut Value, patch: &Value) { } } +fn matches_prefix_filter(key: &str, prefix_filter: Option<&HashSet>) -> bool { + prefix_filter + .map(|prefixes| prefixes.iter().any(|prefix| key.starts_with(prefix))) + .unwrap_or(true) +} + +fn filter_config_keys_by_prefix( + config: Map, + prefix_filter: &HashSet, +) -> Map { + config + .into_iter() + .filter(|(key, _)| matches_prefix_filter(key, Some(prefix_filter))) + .collect() +} + fn get_overrides( query_data: &Map, contexts: &[Context], overrides: &HashMap, merge_strategy: &MergeStrategy, + prefix_filter: Option<&HashSet>, mut on_override_select: Option<&mut dyn FnMut(Context)>, ) -> Result, String> { let mut required_overrides = Map::new(); @@ -157,11 +129,17 @@ fn get_overrides( match merge_strategy { MergeStrategy::REPLACE => { for (key, value) in overriden_value.iter() { + if !matches_prefix_filter(key, prefix_filter) { + continue; + } required_overrides.insert(key.clone(), value.clone()); } } MergeStrategy::MERGE => { for (key, value) in overriden_value.iter() { + if !matches_prefix_filter(key, prefix_filter) { + continue; + } merge( required_overrides .entry(key.as_str()) diff --git a/docs/misc/PERF_ANALYSIS_RESOLUTION.md b/docs/misc/PERF_ANALYSIS_RESOLUTION.md index 6f61798db..552145310 100644 --- a/docs/misc/PERF_ANALYSIS_RESOLUTION.md +++ b/docs/misc/PERF_ANALYSIS_RESOLUTION.md @@ -126,7 +126,7 @@ MergeStrategy::MERGE => { | # | Location | Change | |---|----------|--------| | 1 | `client.rs` / `provider.rs` | Added `CacConfig::get_dimensions()` that clones only the `dimensions` map under the read lock. The provider now fetches dimensions **only when experimentation is enabled** (they are unused otherwise). | -| 2 | `core/config.rs` (`eval_config`, `eval_config_with_reasoning`) | Added a fast path that resolves directly against the **borrowed** `contexts`/`overrides` when there is no prefix filter. Only the rare prefix-filter path builds an owned `Config`. | +| 2 | `core/config.rs` (`eval_config`, `eval_config_with_reasoning`) | Resolves directly against the **borrowed** `contexts`/`overrides` for both unfiltered and prefix-filtered requests. Prefix filtering is applied while collecting override keys, and the already-owned default config is filtered without building a temporary `Config`. | | 3 | `core/config.rs` (`get_overrides`) | Clone a matched `Context` only when a callback actually consumes it; merge override maps by reference instead of cloning them into a temporary `Value::Object`. | All changes preserve existing behavior and semantics; the full @@ -138,8 +138,9 @@ A criterion benchmark was added at `crates/superposition_core/benches/resolve.rs`. It reproduces the reported workload shape — **468,006 contexts, 18 regular dimensions, 18 derived local cohort dimensions, 3 default configs, and 100 query contexts sampled from -actual context conditions** — and compares the optimized path against a -faithful reproduction of the pre-optimization clone behavior. +actual context conditions**. It measures the unfiltered borrowed path and +compares the prefix-filtered borrowed path against a faithful reproduction of +the previous prefix path, which cloned and filtered the complete `Config`. The benchmark intentionally includes local-cohort evaluation: a subset of generated context rules targets derived `lcN` cohort dimensions, while sampled @@ -173,6 +174,29 @@ covered by follow-up benchmarks. > large core hot-path win even when local-cohort evaluation is included. It is > not a claim about end-to-end provider p99 latency. +### Prefix-filter path results + +The prefix benchmark uses the same 468,006-context dataset, 100 rotating +queries, merge strategy, and `config.a` prefix for both implementations. + +| Benchmark | Criterion estimate | Estimate interval | +|-----------|---------------------:|------------------:| +| Unfiltered borrowed reference | 54.674 ms | 53.903–55.856 ms | +| Prefix-filtered, previous cloned path | 904.50 ms | 883.46–927.97 ms | +| Prefix-filtered, optimized borrowed path | **55.535 ms** | 54.544–56.591 ms | + +For the prefix-filtered path, removing the full-config clone reduced the +central estimate by approximately **93.9%**, from 904.50 ms to 55.535 ms, or +about a **16.3× speedup**. The optimized prefix path was approximately 1.6% +slower than the unfiltered borrowed reference in this run, representing the +remaining prefix-matching work. + +Criterion's `change` percentage compares a benchmark with its own saved result +from an earlier run. It does not compare different benchmark names. The +before/after comparison above therefore uses the absolute estimates from +`prefix_pre_optimization_cloned` and `prefix_optimized_borrowed`. These +intervals are Criterion estimates, not p99 latency measurements. + ## Remaining opportunity After removing the clones, the remaining time is dominated by local-cohort