Skip to content

Add CachePilot KVCache benchmark and evaluation suite#2844

Open
imperson123 wants to merge 2 commits into
kvcache-ai:mainfrom
imperson123:add-cachepilot-benchmark
Open

Add CachePilot KVCache benchmark and evaluation suite#2844
imperson123 wants to merge 2 commits into
kvcache-ai:mainfrom
imperson123:add-cachepilot-benchmark

Conversation

@imperson123

Copy link
Copy Markdown

This PR adds CachePilot, a non-invasive KVCache benchmark and evaluation suite for Mooncake.

CachePilot provides:

  • a wrapper for Mooncake official store_kv_bench.py;
  • real Mooncake Store verify_write and read_perf benchmark support;
  • prefix reuse workload evaluation;
  • retrieval scheduler strategy evaluation;
  • CSV/log/figure generation and Streamlit dashboard.

The benchmark does not modify Mooncake core components and is placed under benchmarks/cachepilot.

Basic usage

cd benchmarks/cachepilot
python3 -m venv .venv
source .venv/bin/activate
pip install -r requirements.txt
bash scripts/run_all.sh

Non-invasive evaluation suite under benchmarks/cachepilot wrapping
official store_kv_bench.py with prefix reuse and retrieval scheduler
evaluation, plus Streamlit dashboard.

Co-authored-by: Cursor <cursoragent@cursor.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces CachePilot, a non-invasive evaluation suite for Mooncake's KVCache. It includes wrappers for running official store benchmarks, prefix cache reuse simulations, retrieval scheduler simulations, and a Streamlit dashboard for visualization. The feedback highlights a hardcoded Python path in the Windows batch script that hinders portability, as well as logic bugs and redundant code in the store runner script regarding the default object count handling for the write verification scenario.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +6 to +7
echo [.venv not found] Creating with D:\python.exe ...
D:\python.exe -m venv .venv

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The script contains a hardcoded absolute path to D:\\python.exe when creating the virtual environment. This is highly specific to a single local machine and will fail on almost all other Windows environments. It is recommended to use the standard python command instead.

  echo [.venv not found] Creating with python ...\n  python -m venv .venv

Comment on lines +361 to +364
nr_objects = args.nr_objects
if args.scenario == "verify_write" and args.nr_objects == 64:
# Keep user override if they set --nr-objects; default path uses 16 in scripts
pass

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is a logic bug in how nr_objects is handled for the verify_write scenario. The code uses pass when args.nr_objects == 64, which does nothing and leaves nr_objects as 64. Combined with the unconditional overwrite on lines 387-390, the intended default of 16 is never applied when running the script directly without explicitly passing --nr-objects. Setting nr_objects = 16 here fixes this issue.

    nr_objects = args.nr_objects\n    if args.scenario == \"verify_write\" and args.nr_objects == 64:\n        # Default to 16 for verify_write if not explicitly overridden\n        nr_objects = 16

Comment on lines +387 to +390
effective_nr = 16 if args.scenario == "verify_write" else nr_objects
# Allow explicit --nr-objects to win when user set it in verify script
if args.scenario == "verify_write":
effective_nr = args.nr_objects

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

With the fix applied to lines 361-364 to correctly set nr_objects = 16 as the default for verify_write, this block can be simplified to directly assign effective_nr = nr_objects, removing the redundant and buggy conditional overwrite.

Suggested change
effective_nr = 16 if args.scenario == "verify_write" else nr_objects
# Allow explicit --nr-objects to win when user set it in verify script
if args.scenario == "verify_write":
effective_nr = args.nr_objects
effective_nr = nr_objects

Use PATH python for venv creation and apply a clear default of 16 objects for verify_write when --nr-objects is left at the global default.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant