Add resource/throughput benchmark metrics and offline CI smoke coverage - #99
Merged
luis-gasparschroeder merged 7 commits intoJul 30, 2026
Merged
Conversation
Track CPU/memory/GPU utilization and queries/tokens-per-second throughput alongside the existing hit-rate/latency/accuracy metrics, write results to CSV as well as JSON, and add fully offline unit/integration tests so the instrumentation reruns on every commit without needing a full-scale, paid benchmark run in CI. Also commits a sample results log as documented in benchmarks/ReadMe.md.
benchmarks.benchmark.Benchmark subclasses unittest.TestCase and defines test_run_benchmark(self, max_samples), an existing quirk unrelated to this PR. Importing it at module level in a test_*.py file made pytest collect and try to run it as its own bare test case, which fails because pytest has no way to supply the required `vcache` constructor argument. Moving the import inside setUp() keeps it out of the module's top-level namespace so pytest's collector never sees it.
Contributor
Author
luis-gasparschroeder
requested changes
Jul 22, 2026
luis-gasparschroeder
left a comment
luis-gasparschroeder
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for contributing! Before merging, could you address a few comments:
- the precomputed QPS/TPS currently measures harness speed rather than simulated model latency,
- psutil is undeclared (imported in benchmarks/common/resource_metrics.py, but it is not listed under [project].dependencies or [tool.poetry.group.benchmarks.dependencies])
- NVML is repeatedly initialized without shutdown, and
- the smoke test leaks policy workers because VCache(..., VerifiedDecisionPolicy(...)) starts a background thread and a ThreadPoolExecutor. The smoke test only shuts down the eviction policy.
- The tests should also verify exact metric calculations, not only their presence
- Compute QPS/TPS from summed per-query latency instead of the benchmark loop's wall-clock time, which was dominated by harness overhead rather than simulated model latency - Initialize NVML once and cache the device handle instead of calling nvmlInit() on every sample; shut it down via atexit - Declare psutil as a project dependency (was used but undeclared) - Shut down VerifiedDecisionPolicy's executor/callback thread in the benchmark smoke test's tearDown, fixing a background-thread leak - Strengthen resource metrics and smoke tests to assert exact computed values instead of just presence/bounds
Contributor
Author
|
@luis-gasparschroeder DONE ! |
CostAwareEvictionPolicy never received a meaningful cost value during benchmark runs: cost was measured as wall-clock time around BenchmarkInferenceEngine.create(), which returns its pre-set response instantly, so every cache miss recorded a near-zero cost regardless of the dataset's actual recorded LLM latency. - BenchmarkInferenceEngine.set_next_response() now accepts an optional cost, exposed as next_cost. - VerifiedDecisionPolicy prefers the inference engine's reported cost over wall-clock measurement when available, falling back to the original wall-clock timing otherwise (no behavior change for live engines). - benchmark.py now passes the dataset's recorded LLM generation latency through as the injected cost on cache misses. - Added mean/p95/p99 latency and a hit_rate field to the results JSON, alongside the existing throughput metrics. - Added a CostAwareEvictionPolicy vs. LRUEvictionPolicy comparison to RUN_COMBINATIONS: same dataset/model/sample count, with max_size set low enough to actually trigger eviction (unlike the existing combinations, which keep max_size far above sample count). LRU (not MRU) is the correct baseline since CostAwareEvictionPolicy reduces to plain LRU at cost_weight=0.
…tests - RUN_COMBINATIONS now compares CostAwareEvictionPolicy against LRU, MRU, FIFO, and SCU (not just LRU), all on the same dataset/model/sample count/max_size, so the benchmark reports how cost-aware eviction stacks up against every eviction strategy in the codebase. - Added deterministic unit tests for CostAwareEvictionPolicy covering the cost_weight=0/1 extremes (equivalence to LRU / pure cost-driven eviction), invalid cost_weight clamping, direct validation of the normalization/priority formulas, and negative/zero-cost inputs.
CI's ruff format --check step was failing on the two _generate_and_measure_cost() call sites in process_request() (added in the cost-signal fix) because they exceeded the line-length that would have collapsed them onto one line. Reformatted with ruff 0.11.7 (the version pinned in pyproject.toml/poetry.lock) to match CI exactly.
luis-gasparschroeder
previously approved these changes
Jul 27, 2026
luis-gasparschroeder
left a comment
luis-gasparschroeder
left a comment
Collaborator
There was a problem hiding this comment.
Please remove the unnecessary comments, and the PR is good to be merged.
Addresses luis-gasparschroeder's remaining review comment: drop the SCU-baseline aside and throughput-derivation comment in benchmark.py, and trim the CI rationale paragraph in ReadMe.md.
luis-gasparschroeder
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.