Skip to content

perf(core): avoid config clones in prefix-filtered resolution #1091

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
sauraww wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf(core): avoid config clones in prefix-filtered resolution #1091

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
92 changes: 62 additions & 30 deletions crates/superposition_core/benches/resolve.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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).
Expand All @@ -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.
Expand Down Expand Up @@ -235,45 +236,67 @@ fn build_dataset(num_contexts: usize) -> Dataset {
}
}

/// Current implementation: resolve directly against the borrowed data.
fn resolve_optimized(ds: &Dataset, query: &Map<String, Value>) -> Map<String, Value> {
fn resolve_prefix_optimized(
ds: &Dataset,
query: &Map<String, Value>,
) -> Map<String, Value> {
eval_config(
ds.default_config.clone(),
&ds.contexts,
&ds.overrides,
&ds.dimensions,
query,
MergeStrategy::MERGE,
None,
Some(
PREFIX_FILTER
.iter()
.map(|prefix| prefix.to_string())
.collect(),
),
)
.expect("resolve")
}

/// Pre-optimization behavior: every resolve deep-cloned the full context set
/// twice — once while extracting dimensions, once while building a throwaway
/// `Config` for (unused) prefix filtering.
fn resolve_pre_optimization(
fn resolve_prefix_pre_optimization(
ds: &Dataset,
query: &Map<String, Value>,
) -> Map<String, Value> {
// (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::<HashSet<_>>();

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<String, Value>) -> Map<String, Value> {
eval_config(
ds.default_config.clone(),
&ds.contexts,
&ds.overrides,
&ds.dimensions,
query,
MergeStrategy::MERGE,
Expand Down Expand Up @@ -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))
})
});

Expand Down
114 changes: 46 additions & 68 deletions crates/superposition_core/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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<HashSet<String>> = filter_prefixes
.filter(|prefixes| !prefixes.is_empty())
.map(HashSet::from_iter);
Comment on lines +22 to +24

let overrides_map: Map<String, Value> = 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(
Expand All @@ -77,46 +52,26 @@ pub fn eval_config_with_reasoning(
) -> Result<Map<String, Value>, 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<HashSet<String>> = 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) {
Expand All @@ -135,11 +90,28 @@ pub fn merge(doc: &mut Value, patch: &Value) {
}
}

fn matches_prefix_filter(key: &str, prefix_filter: Option<&HashSet<String>>) -> bool {
prefix_filter
.map(|prefixes| prefixes.iter().any(|prefix| key.starts_with(prefix)))
.unwrap_or(true)
}

fn filter_config_keys_by_prefix(
config: Map<String, Value>,
prefix_filter: &HashSet<String>,
) -> Map<String, Value> {
config
.into_iter()
.filter(|(key, _)| matches_prefix_filter(key, Some(prefix_filter)))
.collect()
}

fn get_overrides(
query_data: &Map<String, Value>,
contexts: &[Context],
overrides: &HashMap<String, Overrides>,
merge_strategy: &MergeStrategy,
prefix_filter: Option<&HashSet<String>>,
mut on_override_select: Option<&mut dyn FnMut(Context)>,
) -> Result<Map<String, Value>, String> {
let mut required_overrides = Map::new();
Expand All @@ -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;
}
Comment on lines 129 to +142
merge(
required_overrides
.entry(key.as_str())
Expand Down
30 changes: 27 additions & 3 deletions docs/misc/PERF_ANALYSIS_RESOLUTION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading