Repository navigation
test: add benchmarks - #6728
test: add benchmarks#6728
Conversation
commit: |
📝 WalkthroughWalkthroughAdds a Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/bench/tv.bench.ts (1)
59-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove ephemeral PR references from comments.
Referencing specific PR numbers (like "PR 2" and "PR 3") in code comments becomes stale and loses context over time once the code is merged. It is generally better practice to describe the architectural goal or the specific optimization being measured.
test/bench/tv.bench.ts#L59-L61: Update the comment to explain the optimization directly (e.g., the overhead saved by hoisting the factory build out of the variant-prop computed) instead of referencing PR 2.test/bench/tv.bench.ts#L72-L74: Update the comment to describe the performance overhead targets (wrapper closure allocation and replacer scan) directly instead of referencing PR 3.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/bench/tv.bench.ts` around lines 59 - 61, Remove the ephemeral PR references from the benchmark comments in test/bench/tv.bench.ts: update lines 59-61 to describe the overhead saved by hoisting factory construction out of the variant-prop recomputation, and update lines 72-74 to describe the targeted wrapper-closure allocation and replacer-scan overhead directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/benchmark.yml:
- Around line 25-35: Update the “Checkout” step using actions/checkout@v7 to set
persist-credentials to false, leaving the remaining workflow steps unchanged.
In `@test/bench/components.bench.ts`:
- Around line 26-30: Update the “toggle loading” benchmark to perform a
deterministic full state cycle on every iteration: set loading to true, await
wrapper.setProps, then set it to false and await wrapper.setProps again. Ensure
each iteration ends in the initial false state instead of relying on the shared
loading toggle.
---
Nitpick comments:
In `@test/bench/tv.bench.ts`:
- Around line 59-61: Remove the ephemeral PR references from the benchmark
comments in test/bench/tv.bench.ts: update lines 59-61 to describe the overhead
saved by hoisting factory construction out of the variant-prop recomputation,
and update lines 72-74 to describe the targeted wrapper-closure allocation and
replacer-scan overhead directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e37f29e-3bbb-4182-b2f7-9c0264d0070d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.github/workflows/benchmark.ymlpackage.jsontest/bench/components.bench.tstest/bench/tv.bench.tsvitest.config.ts
❓ Type of change
📚 Description
Adds a benchmark suite (
test/bench/) covering thetv()factory build vs invocation, slot invocation, and component mount/re-render.📝 Checklist