Skip to content

feat: improve mooncake_bench sweep logic and throughput accounting#6631

Merged
PeaBrane merged 1 commit into
mainfrom
rupei/mooncake-bench-improvements
Feb 26, 2026
Merged

feat: improve mooncake_bench sweep logic and throughput accounting#6631
PeaBrane merged 1 commit into
mainfrom
rupei/mooncake-bench-improvements

Conversation

@PeaBrane

@PeaBrane PeaBrane commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reduce default num_gpu_blocks from 1M to 16K to stress cache eviction
  • Add count_events flag so indexers that don't support remove aren't credited for silently dropped events in throughput numbers
  • Sweep high-to-low for multi-threaded indexers with early stop at 95% sustained throughput; low-to-high for single-threaded with early stop at 5x saturation
  • Export sweep results to JSON alongside the plot
  • Simplify dynamo_mocker import paths

Test plan

  • Run cargo bench --bench mooncake_bench single-indexer mode and verify throughput numbers exclude events for non-remove indexers
  • Run sweep mode and verify early stopping triggers correctly for both multi-threaded and single-threaded indexers
  • Verify JSON output is written alongside the plot file

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Enhanced benchmark sweep reporting with JSON output generation.
    • Implemented multi-threaded and single-threaded benchmark modes with distinct stopping conditions.
  • Chores

    • Updated default GPU block count configuration.
    • Improved event counting in benchmark calculations.

- Reduce default num_gpu_blocks from 1M to 16K to stress cache eviction
- Add count_events flag so indexers that don't support remove aren't
  credited for silently dropped events in throughput numbers
- Sweep high-to-low for multi-threaded indexers with early stop at 95%
  sustained throughput; low-to-high for single-threaded with early stop
  at 5x saturation
- Export sweep results to JSON alongside the plot
- Simplify dynamo_mocker import paths

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Signed-off-by: PeaBrane <[email protected]>
@PeaBrane PeaBrane requested a review from a team as a code owner February 26, 2026 18:27
PeaBrane added a commit that referenced this pull request Feb 26, 2026
Bench improvements extracted to rupei/mooncake-bench-improvements (#6631).

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Signed-off-by: PeaBrane <[email protected]>
@PeaBrane PeaBrane changed the title bench: improve mooncake_bench sweep logic and throughput accounting feat: improve mooncake_bench sweep logic and throughput accounting Feb 26, 2026
@github-actions github-actions Bot added the feat label Feb 26, 2026
@coderabbitai

coderabbitai Bot commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Benchmark configuration enhancements that add conditional multi-threaded vs. single-threaded sweep behavior with custom early stopping criteria, event counting controls, new JSON serialization output, and an adjusted default GPU block count parameter.

Changes

Cohort / File(s) Summary
Benchmark sweep logic
lib/kv-router/benches/mooncake_bench.rs
Introduces supports_remove() and is_multi_threaded() helpers on IndexerArgs. Implements dual-mode sweep behavior with distinct early stopping conditions (95% throughput achievement for multi-threaded, 5x throughput ratio for single-threaded). Adds count_events parameter to run_benchmark() to control event contribution in throughput calculations.
Output and serialization
lib/kv-router/benches/mooncake_bench.rs
Adds SweepStepResult struct with serde flatten and Serialize derive to BenchmarkResults. Enhances JSON output to map sweep results by indexer name with throughput and latency metrics.
Configuration adjustments
lib/kv-router/benches/mooncake_bench.rs
Reduces default GPU block count from 1048576 to 16384. Reorganizes dynamo_mocker imports for clarity.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 Hops through benchmarks with glee,
Multi-threaded sweeps dancing free,
Early stopping conditions aligned,
JSON metrics, carefully designed,
Performance metrics, clear to see!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description covers all required template sections with clear details on changes, test plan, and related context, though it does not explicitly reference a GitHub issue.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title directly summarizes the main changes: improving mooncake_bench sweep logic and throughput accounting, which aligns with the core objectives of refactoring sweep behavior and adding event-counting logic.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/kv-router/benches/mooncake_bench.rs`:
- Around line 1168-1171: Replace fragile substring replacements on
args.sweep_output with path-based extension replacement: use
std::path::Path::with_extension (e.g.
Path::new(&args.sweep_output).with_extension("json").to_string_lossy().into_owned())
to build json_path from args.sweep_output instead of .replace(".png", ...) /
.replace(".svg", ...); apply the same change at the other occurrence mentioned
(around line with the second replacement).
- Around line 1079-1084: The sweep generation code uses args.sweep_steps as n
and divides by (n - 1), which is invalid when sweep_steps <= 1; update the logic
around n and the durations_low_to_high computation to guard against degenerate
inputs by checking args.sweep_steps (or n) first: if n <= 1 produce a sensible
single-entry durations vector (e.g., one value derived from log_min/log_max or
return an error/early exit), otherwise perform the existing interpolation using
(n - 1); ensure this change touches the block that computes
durations_low_to_high and any callers that expect multiple steps so downstream
throughput math/plotting remains stable.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab63b8b and 679fe01.

📒 Files selected for processing (1)
  • lib/kv-router/benches/mooncake_bench.rs

Comment thread lib/kv-router/benches/mooncake_bench.rs
Comment thread lib/kv-router/benches/mooncake_bench.rs
@PeaBrane PeaBrane merged commit 824b456 into main Feb 26, 2026
88 of 89 checks passed
@PeaBrane PeaBrane deleted the rupei/mooncake-bench-improvements branch February 26, 2026 20:09
yao531441 pushed a commit to yao531441/dynamo that referenced this pull request May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants