ci(benchmark/react): force perf_controller_actor_ to use ui task runner in frozen mode#1657
Conversation
…nner in frozen mode To compare: lynx-family/lynx@develop...hzy:lynx:p/hzy/benchx_cli
|
📝 WalkthroughWalkthroughUpdated the PICK_COMMIT constant in packages/lynx/benchx_cli/scripts/build.mjs to a new hash, altering which upstream commit is cherry-picked during the build. No other logic or exports changed. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/lynx/benchx_cli/scripts/build.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
packages/lynx/benchx_cli/scripts/build.mjs
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
packages/lynx/benchx_cli/scripts/build.mjs
🔇 Additional comments (2)
packages/lynx/benchx_cli/scripts/build.mjs (2)
115-115: Confirm frozen mode configuration aligns with PR objectives.The build arguments include
enable_frozen_mode=truewhich aligns with the PR title mentioning "frozen mode". This suggests the commit being cherry-picked contains changes that affect behavior specifically when frozen mode is enabled.
99-102: Verify cherry-pick will apply cleanly before running the script.
Simulate the cherry-pick in an isolated worktree or as a patch to avoid conflicts in your main workspace:
- Worktree approach:
git fetch origin git worktree add /tmp/tw TARGET-branch cd /tmp/tw git cherry-pick <PICK_COMMIT> --no-commit # exit code 0 = no conflicts cd -; git worktree remove /tmp/tw --force; git worktree prune- Patch check approach:
git format-patch -1 <PICK_COMMIT> --stdout \ | git apply --3way --check - # exit code 0 = clean applyEnsure one of these exits with status 0 before merging the updated commit into your release.
|
|
||
| const COMMIT = 'd6dd806293012c62e5104ad7ed2bed5c66f4f833'; | ||
| const PICK_COMMIT = '78493ae8581046275f6333ce16004666560ce058'; | ||
| const PICK_COMMIT = 'ff0c7dbe93ddf526c3a2b814215797ceeb3bb085'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Update commit hash without documentation - verify the change is intentional.
The PICK_COMMIT constant has been updated from 78493ae8581046275f6333ce16004666560ce058 to ff0c7dbe93ddf526c3a2b814215797ceeb3bb085. This change aligns with the PR title mentioning forcing perf_controller_actor_ to use UI task runner in frozen mode, but there's no documentation about what this specific commit contains.
Please verify that this commit hash update is correct by running the following script to check what changes are included in the new commit:
🏁 Script executed:
#!/bin/bash
# Description: Verify the commit hash and examine the changes it introduces
# Expected: The commit should contain changes related to perf_controller_actor_ and UI task runner
# Check if the new commit exists in the hzy/lynx repository
curl -s "https://api.github.com/repos/hzy/lynx/commits/ff0c7dbe93ddf526c3a2b814215797ceeb3bb085" | jq -r '.commit.message, .commit.author.date'
# Also check what the old commit was about for comparison
curl -s "https://api.github.com/repos/hzy/lynx/commits/78493ae8581046275f6333ce16004666560ce058" | jq -r '.commit.message, .commit.author.date'Length of output: 1209
Correct PICK_COMMIT to the intended perf_controller_actor_ change.
Both the old (78493ae8581046275f6333ce16004666560ce058) and new (ff0c7dbe93ddf526c3a2b814215797ceeb3bb085) hashes reference the benchx_cli introduction commit from 2025-08-22, not the perf_controller_actor_ UI task-runner update mentioned in the PR. Update the hash at packages/lynx/benchx_cli/scripts/build.mjs:33 to the correct commit or document why this benchx_cli commit is being picked.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
React Example#4850 Bundle Size — 237.5KiB (0%).254d397(current) vs 9e2c497 main#4838(baseline) Bundle metrics
|
| Current #4850 |
Baseline #4838 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
160 |
160 |
|
65 |
65 |
|
45.83% |
45.83% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #4850 |
Baseline #4838 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.74KiB |
91.74KiB |
Bundle analysis report Branch hzy:p/hzy/bench_7 Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#4841 Bundle Size — 367.43KiB (0%).254d397(current) vs 9e2c497 main#4829(baseline) Bundle metrics
Bundle size by type
|
| Current #4841 |
Baseline #4829 |
|
|---|---|---|
235.43KiB |
235.43KiB |
|
100.16KiB |
100.16KiB |
|
31.84KiB |
31.84KiB |
Bundle analysis report Branch hzy:p/hzy/bench_7 Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Pull Request Overview
This PR updates the commit hash used by the benchmark CLI build script to ensure the perf_controller_actor_ uses the UI task runner in frozen mode for more accurate benchmarking results.
- Updates the
PICK_COMMITconstant to reference a newer commit that contains the necessary performance controller changes
CodSpeed Performance ReportMerging #1657 will degrade performances by 12.77%Comparing Summary
Benchmarks breakdown
|
|
|
||
| const COMMIT = 'd6dd806293012c62e5104ad7ed2bed5c66f4f833'; | ||
| const PICK_COMMIT = '78493ae8581046275f6333ce16004666560ce058'; | ||
| const PICK_COMMIT = 'ff0c7dbe93ddf526c3a2b814215797ceeb3bb085'; |
There was a problem hiding this comment.
I am wondering if we can use branch name instead of commit id. If we use branch name, we always use the first commit of the branch as PICK_COMMIT, then we will never need update this file on any updates.
To compare: lynx-family/lynx@develop...hzy:lynx:p/hzy/benchx_cli
Summary by CodeRabbit
Checklist