Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
colinaaa merged 1 commit into from
Sep 9, 2025
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/lynx/benchx_cli/scripts/build.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ console.log('noop')
}

const COMMIT = 'd6dd806293012c62e5104ad7ed2bed5c66f4f833';
const PICK_COMMIT = 'ff0c7dbe93ddf526c3a2b814215797ceeb3bb085';
const PICK_COMMIT = '3d75a38c2e5b422da9b32851a1bd5dfe25ca8ed6';
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Sep 9, 2025

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.


function checkCwd() {
try {
Expand Down
Loading