Skip to content

ci(benchmark/react): force LynxNormalTask and LynxHighTask runs on ui thread in frozen mode#1693

Merged
colinaaa merged 1 commit intolynx-family:mainfrom
hzy:p/hzy/bench_9
Sep 9, 2025
Merged

ci(benchmark/react): force LynxNormalTask and LynxHighTask runs on ui thread in frozen mode#1693
colinaaa merged 1 commit intolynx-family:mainfrom
hzy:p/hzy/bench_9

Conversation

@hzy
Copy link
Copy Markdown
Collaborator

@hzy hzy commented Sep 9, 2025

Summary by CodeRabbit

  • Chores
    • Updated internal build configuration to align with the latest upstream source, improving consistency and reproducibility of builds.
    • Ensures the integration process remains current without altering app behavior or features.
    • No user-facing changes; functionality and UI remain unchanged.

close: #1694

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).
  • Changeset added, and when a BREAKING CHANGE occurs, it needs to be clearly marked (or not required).

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Sep 9, 2025

⚠️ No Changeset found

Latest commit: b4b67bc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 9, 2025

📝 Walkthrough

Walkthrough

Updates the PICK_COMMIT hash in packages/lynx/benchx_cli/scripts/build.mjs to a new commit ID. No other build steps or constants changed.

Changes

Cohort / File(s) Change summary
Benchx CLI build script
packages/lynx/benchx_cli/scripts/build.mjs
Updated PICK_COMMIT from ff0c7d... to 3d75a3...; no other modifications.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Suggested labels

framework:React

Suggested reviewers

  • upupming
  • colinaaa

Poem

I twitch my whiskers, hop—click, pick!
A fresher hash, a cherry-pick.
The build script hums, the branches sing,
One tiny change—commit takes wing.
Carrot-shaped logs, green lights glow,
Onward we bound—release, let’s go! 🥕✨

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

Pre-merge checks (1 passed, 1 warning, 1 inconclusive)

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ❓ Inconclusive The current pull request description consists solely of the default template and checklist and does not provide any details about the actual changes made, making it too generic to assess whether it corresponds to the modifications in the CI benchmarks or the commit hash update. Please update the pull request description to include a concise summary of the changes, such as the updated commit hash in build.mjs and the adjustment to force LynxNormalTask and LynxHighTask on the UI thread in frozen mode.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title concisely and clearly describes the primary change of the pull request—forcing LynxNormalTask and LynxHighTask to run on the UI thread in frozen mode—without unnecessary details or noise, making it easy for reviewers to understand the main objective of the change at a glance.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ec4537 and b4b67bc.

📒 Files selected for processing (1)
  • packages/lynx/benchx_cli/scripts/build.mjs (1 hunks)


const COMMIT = 'd6dd806293012c62e5104ad7ed2bed5c66f4f833';
const PICK_COMMIT = 'ff0c7dbe93ddf526c3a2b814215797ceeb3bb085';
const PICK_COMMIT = '3d75a38c2e5b422da9b32851a1bd5dfe25ca8ed6';
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.

🛠️ Refactor suggestion

⚠️ Potential issue

Stale binary risk: changing only PICK_COMMIT won’t trigger a rebuild.

checkBinary() keys the cache solely on COMMIT, and the commit file is populated from git rev-parse HEAD. Because you cherry-pick with -n (no commit), HEAD remains COMMIT. Result: updating PICK_COMMIT can be silently ignored if dist/bin/benchx_cli already exists with the same base COMMIT. CI may keep using a binary that does not include the newly picked changes.

Fix by incorporating both COMMIT and PICK_COMMIT into the cache fingerprint and into the commit file.

Apply this diff:

 const COMMIT = 'd6dd806293012c62e5104ad7ed2bed5c66f4f833';
-const PICK_COMMIT = '3d75a38c2e5b422da9b32851a1bd5dfe25ca8ed6';
+const PICK_COMMIT = '3d75a38c2e5b422da9b32851a1bd5dfe25ca8ed6';
+const BUILD_FINGERPRINT = `${COMMIT}:${PICK_COMMIT}`;

 async function checkBinary() {
   if (
     existsSync('./dist/bin/benchx_cli')
     && existsSync('./dist/bin/benchx_cli.commit')
   ) {
     const { exitCode, stdout } = await $`cat ./dist/bin/benchx_cli.commit`;
-    if (exitCode === 0 && stdout.trim() === COMMIT) {
+    if (exitCode === 0 && stdout.trim() === BUILD_FINGERPRINT) {
       return true;
     }
   }
   return false;
 }

 // build from source
 await $`
   cd lynx
   source tools/envsetup.sh
   gn gen --args='enable_unittests=true enable_trace="perfetto" jsengine_type="quickjs" enable_frozen_mode=true' out/Default
   ninja -C out/Default benchx_cli
   mkdir -p ../dist/bin
   cp ${
     process.platform === 'darwin'
       ? 'out/Default/benchx_cli'
       : 'out/Default/exe.unstripped/benchx_cli' // linux
   } ../dist/bin/benchx_cli
-  git rev-parse HEAD > ../dist/bin/benchx_cli.commit
+  echo ${BUILD_FINGERPRINT} > ../dist/bin/benchx_cli.commit
   rm -rf out
 `.pipe(process.stdout);

Optionally, drop -n from git cherry-pick so HEAD advances, but you’d still want the composite fingerprint to cover changes beyond the base commit.

Also applies to: 53-64, 118-124

🤖 Prompt for AI Agents
In packages/lynx/benchx_cli/scripts/build.mjs around lines 33 (and also apply
same change to ranges 53-64 and 118-124), the script currently only fingerprints
and caches the binary by COMMIT (from git rev-parse HEAD) so changing
PICK_COMMIT won’t force a rebuild; update the cache key and the commit-file
contents to include both COMMIT and PICK_COMMIT (e.g. composite fingerprint like
`${COMMIT}:${PICK_COMMIT}`) wherever checkBinary/read/write commit-file logic
runs so the presence of a different PICK_COMMIT invalidates the cache and forces
rebuild; also, when performing the cherry-pick consider removing the `-n` flag
so HEAD advances (optional) but still ensure the composite fingerprint is
written to disk and used for cache checks.

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@relativeci
Copy link
Copy Markdown

relativeci Bot commented Sep 9, 2025

React Example

#5008 Bundle Size — 238.2KiB (0%).

b4b67bc(current) vs de8a1c7 main#5005(baseline)

Bundle metrics  no changes
                 Current
#5008
     Baseline
#5005
No change  Initial JS 0B 0B
No change  Initial CSS 0B 0B
No change  Cache Invalidation 0% 0%
No change  Chunks 0 0
No change  Assets 4 4
No change  Modules 163 163
No change  Duplicate Modules 67 67
No change  Duplicate Code 46.88% 46.88%
No change  Packages 2 2
No change  Duplicate Packages 0 0
Bundle size by type  no changes
                 Current
#5008
     Baseline
#5005
No change  IMG 145.76KiB 145.76KiB
No change  Other 92.45KiB 92.45KiB

Bundle analysis reportBranch hzy:p/hzy/bench_9Project dashboard


Generated by RelativeCIDocumentationReport issue

@relativeci
Copy link
Copy Markdown

relativeci Bot commented Sep 9, 2025

Web Explorer

#5001 Bundle Size — 367.39KiB (+0.25%).

b4b67bc(current) vs de8a1c7 main#4998(baseline)

Bundle metrics  Change 2 changes
                 Current
#5001
     Baseline
#4998
No change  Initial JS 144.22KiB 144.22KiB
No change  Initial CSS 31.84KiB 31.84KiB
Change  Cache Invalidation 10.03% 0%
No change  Chunks 8 8
No change  Assets 8 8
Change  Modules 219(-0.45%) 220
No change  Duplicate Modules 16 16
No change  Duplicate Code 3.33% 3.33%
No change  Packages 4 4
No change  Duplicate Packages 0 0
Bundle size by type  Change 2 changes Regression 1 regression Improvement 1 improvement
                 Current
#5001
     Baseline
#4998
Improvement  JS 235.39KiB (-0.08%) 235.59KiB
Regression  Other 100.16KiB (+1.11%) 99.06KiB
No change  CSS 31.84KiB 31.84KiB

Bundle analysis reportBranch hzy:p/hzy/bench_9Project dashboard


Generated by RelativeCIDocumentationReport issue

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Sep 9, 2025

CodSpeed Performance Report

Merging #1693 will improve performances by 12.39%

Comparing hzy:p/hzy/bench_9 (b4b67bc) with main (8ec4537)

🎉 Hooray! codspeed-cpp just leveled up to 1.2.0!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

⚡ 1 improvements
✅ 42 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
basic-performance-nest-level-100 6.6 ms 5.9 ms +12.39%

@hzy hzy enabled auto-merge (squash) September 9, 2025 13:39
@colinaaa colinaaa merged commit da014a9 into lynx-family:main Sep 9, 2025
90 of 97 checks passed
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.

Merging #1680 will **degrade performances by 13.01%**

2 participants