Add Benchmark section to command reference pages - #550
Conversation
Adds a shared Benchmark component (throughput bars + methodology table) and wires it into the 16 command pages that have published results in dragonflydb/benchmarking: GET, SET, HGET, HSET, MGET, MSET, ZADD, ZSCORE, DEL, EXPIRE, INCR, LPOP, LPUSH, RPOP, RPUSH, TTL. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Summary by QodoAdd benchmark sections to command reference pages
AI Description
Diagram
High-Level Assessment
Files changed (18)
|
Code Review by Qodo
1. Zero-ops division bug
|
| const width = ((engine.ops / scale) * 100).toFixed(1) + "%"; | ||
| const relative = engine.isWinner | ||
| ? "OPS" | ||
| : (dragonflyOps / engine.ops).toFixed(1) + "× OPS"; |
There was a problem hiding this comment.
1. Zero-ops division bug 🐞 Bug ☼ Reliability
Benchmark can render invalid widths/ratios (NaN%, Infinity×) because it divides by scale and engine.ops without guarding against zero values. This yields broken visual output if any throughput prop is 0 (or all are 0).
Agent Prompt
### Issue description
`Benchmark` computes bar widths and relative ratios via division by `scale` and `engine.ops` without validating that they are > 0. If any input is 0 (or all are 0), the component will generate invalid CSS widths (e.g. `NaN%`) and/or display ratios like `Infinity× OPS`.
### Issue Context
This component is now shared across multiple docs pages; even if current pages pass only positive values, future additions/edits (or partial/placeholder data) can easily introduce zeros.
### Fix Focus Areas
- src/components/Benchmark/index.tsx[56-58]
- src/components/Benchmark/index.tsx[80-84]
### Suggested fix
- Ensure `scale` is never 0 (e.g., `const scale = max > 0 ? ... : 1;`) and consider clamping negative values.
- When computing `relative`, handle `engine.ops <= 0` explicitly (e.g., show `"—"` instead of dividing).
- Optionally, short-circuit rendering with a “No benchmark data” message when `max <= 0`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const engines = [ | ||
| { name: "Dragonfly", ops: dragonflyOps, isWinner: true }, | ||
| { name: "Valkey", ops: valkeyOps, isWinner: false }, | ||
| { name: "Redis", ops: redisOps, isWinner: false }, | ||
| ]; |
There was a problem hiding this comment.
2. Hardcoded winner styling 🐞 Bug ≡ Correctness
Benchmark hardcodes Dragonfly as isWinner: true, so the highlighted “winner” styling can disagree with the actual max throughput if values change or the component is reused. This can mislead readers because the UI implies the highlighted row is the best performer.
Agent Prompt
### Issue description
The component currently encodes “winner” as “Dragonfly” via a hardcoded `isWinner` flag. If future benchmark data ever shows Redis/Valkey higher (or if a typo swaps values), the component will still style Dragonfly as the winner.
### Issue Context
`max` is already computed, but not used to determine the winner. The `isWinner` flag also controls both the highlighted styles and the `relative` label logic.
### Fix Focus Areas
- src/components/Benchmark/index.tsx[50-57]
- src/components/Benchmark/index.tsx[81-90]
### Suggested fix
- Compute winner/leader dynamically from `max` (and define tie behavior), e.g. set `isWinner: ops === maxOps`.
- If you still want Dragonfly always visually distinct, separate concerns:
- `isPrimary` (Dragonfly) for baseline styling/ratio logic, and
- `isLeader` (max ops) for winner styling.
- Update `relative` text logic accordingly so it remains meaningful when Dragonfly is not the leader.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
odedponcz
left a comment
There was a problem hiding this comment.
I see RPS numbers but not latency. Why?
|
@odedponcz you had originally said to just show throughput...I can add latency too |
Renders the harness's per-engine p50/p99/p99.9/avg-latency rows (from each command's Expected results section) as a simple table directly beneath the throughput bars, above Methodology. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds scripts/sync-benchmarks.ts, which fetches the benchmarking repo, parses each command's *_reproduce.md (Stateful setup, Test run flags, and Expected results table), and regenerates the marked <Benchmark> block on the matching command-reference page — auto-picking up new commands as they're added upstream. Idempotent: reruns with unchanged upstream data touch nothing. Wired into .github/workflows/sync-benchmarks.yml, which runs the script daily and opens a PR only when something actually changed, so updates to published performance numbers get reviewed rather than landing silently on every build. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Benchmarkcomponent (throughput bars vs. Redis/Valkey + a methodology table) styled with the docs site's existing design tokens — no colors/typography from the marketing mockup were carried over.GET,SET,HGET,HSET,MGET,MSET,ZADD,ZSCORE,DEL,EXPIRE,INCR,LPOP,LPUSH,RPOP,RPUSH,TTL.*_reproduce.mdin the benchmarking repo (dfly_benchharness,m7g.8xlargeserver /c6gn.8xlargeclient, arm64). No placeholder values.Test plan
yarn docusaurus buildsucceeds with no broken linkstsc --noEmitshows no new errors beyond a pre-existing repo-wideJSXnamespace warning🤖 Generated with Claude Code