test: add Criterion bench suite for PPE hot-path costs

[abdallahsamabd]

Summary

Adds a Criterion benchmark suite for the Praxis Policy Engine hot path (#19).

• New unpublished crate ppe-benches under benches/ covering:
  • hook_overhead — plugin dispatch (noop plugins, no APL/PDP)
  • full_decision — APL plugin_only / cedar_only / plugin_then_cedar
  • throughput — concurrent Tokio callers, including YAML mode: concurrent
  • pdp_cost — Cedar / CEL / OPA evaluate only
  • memory — session-taint growth + optional dhat-heap profile
• make bench runs the suite on demand (not part of make ci)
• Baseline numbers, hardware notes, CPU/memory findings, and CI gate decision in docs/benchmarks.md

Setup (YAML load, Cedar compile) stays outside Criterion iters so timings reflect PolicyEngine::invoke_named / PdpResolver::evaluate / SessionStore, not load cost.

CI policy: do not gate PRs on wall-clock benches. Clippy/make ci still compile all [[bench]] targets so the suite cannot bitrot.

Test plan

• cargo bench -p ppe-benches --no-run
• cargo bench -p ppe-benches -- --test
• cargo clippy -p ppe-benches --all-targets -- -D warnings
• make bench (or targeted: --bench full_decision, --bench pdp_cost)
• Optional: cargo bench -p ppe-benches --features dhat-heap --bench memory
• Confirm docs/benchmarks.md matches local hardware / re-run if publishing release numbers

[praxis-bot]

Criterion Bench Suite for PPE Hot Path

PR adds an ppe-benches crate covering plugin dispatch, full-decision latency, throughput, per-PDP cost, and memory/session-taint growth. Suite design is solid: setup runs outside timed loops, fixtures mirror production wiring, CI compiles the suite via clippy --all-targets without wall-clock gating, and docs/benchmarks.md records the decision rationale and baseline numbers.

Findings

#	Severity	File	Issue
1	Large	benches/memory.rs:77	session_append_one accumulates state across Criterion iterations
2	Medium	benches/pdp_cost.rs:98	PDP evaluate results not checked for expected decision outcome

[praxis-bot]

[Large] session_append_one appends the same "EXTRA" label to the same session key across all Criterion iterations without resetting the store. Two failure modes depending on append_labels semantics:

1. Deduplicates: every iteration after the first measures duplicate-detection cost, not actual append cost. The benchmark is really measuring "attempt to add a label that already exists."
2. Does not deduplicate: the label set grows unboundedly across iterations, making measurements non-stationary -- later iterations are slower than earlier ones.

Either way, the benchmark does not hold label count constant at n_labels as the BenchmarkId parameter implies.

Fix: use a unique label per iteration (e.g., AtomicU64 counter in the closure to generate format!("X{}", counter.fetch_add(1, Ordering::Relaxed))) so each iteration appends a genuinely new label to the baseline set. Alternatively, document the intentional drift and rename the benchmark to reflect what it actually measures.

[abdallahsamabd]

Fixed by appending a unique label per iteration (AtomicU64) so each sample is a genuine append on top of the seeded baseline.

[praxis-bot]

[Medium] .expect("cedar eval") only asserts Ok(...), not the decision outcome. If reader_bag() does not correctly populate the attributes Cedar needs (e.g., if the "role.reader" bag key does not map to principal.roles.contains("reader") in the Cedar entity model), the benchmark silently times the deny path instead of the allow path.

invoke_once in lib.rs guards against exactly this -- its doc says "Criterion must not silently time a deny path when the harness expected allow." The same principle applies here. The same concern applies to cel_evaluate and opa_evaluate below.

Fix: assert the decision is allow, at minimum once during setup before the timed loop begins. A single assert! outside the bench_function closure catches bag misconfiguration before baselines go stale.

[abdallahsamabd]

Added a one-shot Allow assert in setup (outside the timed loop) for Cedar, CEL, and OPA before Criterion starts measuring.

[araujof]

Thanks for this PR!

The suite builds and its smoke tests pass. A few notes to address missing measurements and some other nits:

• Criterion does not report p95 or p99 by default. The docs only record means, so the requested p50/p95/p99 results are not covered.
• No CPU profile was captured; the document gives an expected flamegraph instead.
• session_append_one keeps adding labels to the same store, so the 8/64/512 starting sizes are quickly swamped. It also reports n_labels elements per iteration even though only one label is appended.
• The memory results do not show per-decision allocation or footprint growth with policy size.

Please address these before merging.

[terylt]

Hi @abdallahsamabd, Thank you for the contribution. Here is my review on this PR:

Finding 1: engine_plugins_only registers on the wrong hook, so no plugin is ever dispatched

benches/src/lib.rs:1347-1360

mgr.register_handler::<CmfHook, _>(plugin, noop_config(&name, mode, priority))

register_handler registers under H::NAME (crates/ppe-core/src/registry.rs:271), and CmfHook::NAME is "cmf" (crates/ppe-core/src/cmf/message.rs:235). The benches then invoke on "cmf.tool_pre_invoke". entries_for_hook returns nothing, snapshot.route_annotations is empty because no YAML is loaded on this path, and invoke_named takes the early return at crates/ppe-core/src/engine.rs:1275.

Reproduced by swapping NoopPlugin for a plugin that bumps an AtomicUsize in handle and invoking once through the same fixture:

n=1 mode=Sequential -> hits=0

Zero handlers run. Every hook_overhead case measures the same empty-registry early return, which is why the reported numbers are what they are: 16 no-op plugins at ~304 ns against an empty registry at ~237 ns is about 4 ns per async trait-object dispatch, and that is not a number the executor can produce.

The YAML fixtures are unaffected. They register through NoopFactory, which builds handler names from config.hooks, so they land on the right hook. Same counter, driven through engine_from_yaml:

plugin_only=1  plugin_then_cedar=1  yaml_concurrent=4

So full_decision, pdp_cost, memory, and the two apl_* rows of throughput are measuring real work.

What this invalidates

• hook_overhead in full: sequential/{1,4,16}, concurrent/{1,4,16}, and empty_registry are all the same code path.
• throughput/plugins_sequential and throughput/plugins_concurrent_mode, which build their engines with engine_plugins_only(4, ...). The ~587 Kelem/s reported for plugins_concurrent_mode is the cost of tokio::spawn and join, not of the engine.
• The first two rows of the CPU profile findings table in docs/benchmarks.md, which conclude that plugin dispatch is negligible against PDP cost and that APL orchestration adds 5 to 6 microseconds over bare hook dispatch. Both are computed against a floor that is not measuring dispatch.

Fix

register_handler_for_names::<CmfHook, _>(plugin, config, &[HOOK_TOOL_PRE]), which is the API the CMF pattern expects and which NoopFactory is already effectively using.

The suite should also carry a guard so this cannot recur silently. A bench that measures nothing looks exactly like a bench that measures something fast, and the invoke_once allow-assertion does not catch it because the empty path does return allow. Either assert find_plugin_entries(name).len() in the fixture before the timed loop, or keep the hit counter and assert it once outside the loop.

Finding 2: memory/session_append_one does not measure what its parameter says

benches/memory.rs:896-912

The bench appends a unique label to the same "sess-taint" key on every iteration. The comment explains why the label has to be unique, which is correct as far as it goes, but the consequence is that the label set grows without bound across the measurement. After a few thousand Criterion iterations the seeded size is irrelevant, so the 8 / 64 / 512 in the BenchmarkId is only a starting point and all three cases converge on the same thing, which is append into a set that is large and getting larger.

This is consistent with session_append_one being the one bench in the file with no row in the results table.

iter_batched with a freshly seeded store per batch would measure append at a known set size.

Finding 3: clippy --features dhat-heap fails on the documented command

benches/memory.rs:37

eprintln!("ppe-benches: dhat heap profiler active (--features dhat-heap)");

The workspace sets clippy::print_stderr = "deny" and the file's allow list covers expect_used, unwrap_used, and panic but not this. Confirmed:

error: use of `eprintln!`
  --> benches/memory.rs:37:9
   = note: requested on the command line with `-D clippy::print-stderr`

CI does not catch it, because make lint runs --all-targets without --all-features, so the dhat path is never linted. It bites the first person who follows the dhat instructions in docs/benchmarks.md with clippy in the loop. Either add clippy::print_stderr to the file's allow list with the existing reason, or drop the line, since Criterion already prints the target name.

Finding 4: the session-id row in the results table is inconsistent with its own fixture

docs/benchmarks.md, memory table.

memory/full_decision_with_session_id is reported at ~59.7 microseconds, the same figure to three significant digits as full_decision/cedar_only. That bench uses YAML_PLUGIN_THEN_CEDAR (benches/memory.rs:940), which is reported at ~77.8 microseconds in the latency table, so the session-id case should be at or above that, not below it.

It reads like a copy-paste, and it matters because it feeds the stated conclusion that session hydrate and persist are not the dominant cost for this fixture. That conclusion may well be right, but the number as printed does not support it.

Smaller things

Box::leak for hook names. benches/src/lib.rs:1242 leaks a &'static str per hook name per plugin instantiation. It is bounded and confined to setup, so nothing is harmed, but register_for_names takes &[&str] and does not require 'static, so the leak buys nothing.

group.throughput leaks across benches. In hook_overhead it is set inside the for &n loop and is still in effect for empty_registry, which therefore reports as 16 elements. Same pattern in memory.rs, where full_decision_with_session_id inherits Elements(1000) from the session-count loop. Reporting only, but the elements-per-second column is wrong for those two rows.

black_box(()) is a no-op. It appears in every timed closure. Where the work is already anchored by an assertion or by a returned value that is black-boxed, it is harmless noise; where it is the only thing holding the result, it is not holding anything.

Criterion 0.5.1 is from 2023. 0.7 is current. 0.5 is what drags plotters, and through it wasm-bindgen and web-sys, into the lockfile, along with the duplicate itertools 0.10. Not a blocker given cargo deny passes, but the upgrade is close to free on a new crate with no baselines to preserve.

Package layout. A package directory named benches/ at the workspace root, with bench sources at the package root and explicit path entries, sits oddly next to the existing crates/ and builtins/ convention. crates/ppe-benches/ with sources in its own conventional benches/ subdirectory would need no path entries and would match the tree.

Worth keeping from what the suite already found

The targets that work surface two results that are worth their own issue rather than a line in a table.

Cedar evaluate at ~45 microseconds for a single-policy policy set is an order of magnitude slower than Cedar's is_authorized normally runs. The likely explanation is that the entity store is rebuilt per call. If that is what it is, it is a real hot-path defect and this PR is what found it.

CEL at ~8 microseconds against Cedar at ~45 and OPA at ~54 is an operator-facing result that belongs somewhere more discoverable than a benchmarks appendix.

Neither was investigated here.

Suggested disposition

Request changes on finding 1, which needs the fixture corrected, the affected targets re-run, and the dispatch and CPU-profile sections of docs/benchmarks.md rewritten against real numbers. Finding 2 needs the same treatment on a smaller scale. Findings 3 and 4 are single-line fixes. Everything under "smaller things" is optional and none of it should hold the PR.

[terylt]

Overall, nice work! See my comments in the previous message for suggested changes.