Skip to content

ci(benchmark/react): enable "LYNX_ENABLE_FROZEN_MODE=true"#1623

Merged
colinaaa merged 1 commit intolynx-family:mainfrom
hzy:p/hzy/bench_2
Aug 28, 2025
Merged

ci(benchmark/react): enable "LYNX_ENABLE_FROZEN_MODE=true"#1623
colinaaa merged 1 commit intolynx-family:mainfrom
hzy:p/hzy/bench_2

Conversation

@hzy
Copy link
Copy Markdown
Collaborator

@hzy hzy commented Aug 28, 2025

Summary by CodeRabbit

  • Chores

    • Updated build references to newer upstream revisions to keep alignment with the latest codebase and verification steps.
  • Tests

    • Added a new iterative Fibonacci benchmark entry to the benchmark suite ("more faster fib(20)"), augmenting existing performance comparisons.
  • Notes

    • No functional or UI changes; runtime behavior remains unchanged outside of the updated benchmark and build references.

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).

Copilot AI review requested due to automatic review settings August 28, 2025 12:01
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Aug 28, 2025

⚠️ No Changeset found

Latest commit: f826198

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 Aug 28, 2025

📝 Walkthrough

Walkthrough

Updated two commit hash constants in the BenchX CLI build script and added an iterative Fibonacci implementation plus a new benchmark entry in a React benchmark case.

Changes

Cohort / File(s) Summary
BenchX CLI build script
packages/lynx/benchx_cli/scripts/build.mjs
Updated COMMIT and PICK_COMMIT SHA values to new targets; also added enable_frozen_mode=true to GN gen args for out/Default. No other logic changes.
React benchmark — Fibonacci case
benchmark/react/cases/001-fib/index.ts
Added iterative fib3(n) implementation and inserted a new benchmark entry "more faster fib(20)" that runs fib3(20) before the existing fib2 benchmark.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • colinaaa

Poem

I twitch my whiskers, hashes hop anew,
A loop for numbers, quick steps in two,
Cherry-picks, GN flags, and benchmarks run,
I nibble at code till the morning sun.
Thump-thump—build and bench, all neatly done. 🐇

✨ 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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enables frozen mode for React benchmarks by updating commit hashes and simplifying benchmark command configurations. The changes remove repeat parameters and update commit references to implement frozen mode optimization.

  • Updates commit hashes in the benchmark CLI build script to enable frozen mode
  • Simplifies benchmark commands by removing explicit repeat parameters
  • Standardizes perfetto benchmark execution configuration

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/lynx/benchx_cli/scripts/build.mjs Updates COMMIT and PICK_COMMIT hashes to reference frozen mode implementation
benchmark/react/package.json Removes --repeat parameters from perfetto benchmark commands

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: 2

🧹 Nitpick comments (1)
packages/lynx/benchx_cli/scripts/build.mjs (1)

32-33: Nit: clarify constant intent.

Consider more descriptive names or comments for maintainers skimming the file.

-const COMMIT = 'f557bc907f8eac7d45386d493dea9b808d98dd7d';
-const PICK_COMMIT = '471ebc337ca762e08de0d1e488e21ed79c8107c1';
+// Upstream lynx commit to build from (base)
+const UPSTREAM_LYNX_COMMIT = 'f557bc907f8eac7d45386d493dea9b808d98dd7d';
+// Patch to cherry-pick (applied with -n, not changing HEAD)
+const UPSTREAM_PATCH_COMMIT = '471ebc337ca762e08de0d1e488e21ed79c8107c1';

If you adopt the rename, propagate usages accordingly.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 1a32dd8 and 19f2452.

📒 Files selected for processing (1)
  • packages/lynx/benchx_cli/scripts/build.mjs (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: build / Build (Windows)
  • GitHub Check: build / Build (Ubuntu)
  • GitHub Check: test-rust / Test (Ubuntu)
  • GitHub Check: CodeQL Analyze (javascript-typescript)

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@relativeci
Copy link
Copy Markdown

relativeci bot commented Aug 28, 2025

Web Explorer

#4683 Bundle Size — 367.43KiB (0%).

f826198(current) vs 1a32dd8 main#4681(baseline)

Bundle metrics  Change 1 change
                 Current
#4683
     Baseline
#4681
No change  Initial JS 144.23KiB 144.23KiB
No change  Initial CSS 31.84KiB 31.84KiB
Change  Cache Invalidation 0% 63.22%
No change  Chunks 8 8
No change  Assets 8 8
Change  Modules 218(-0.46%) 219
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  no changes
                 Current
#4683
     Baseline
#4681
No change  JS 235.43KiB 235.43KiB
No change  Other 100.16KiB 100.16KiB
No change  CSS 31.84KiB 31.84KiB

Bundle analysis reportBranch hzy:p/hzy/bench_2Project dashboard


Generated by RelativeCIDocumentationReport issue

@relativeci
Copy link
Copy Markdown

relativeci bot commented Aug 28, 2025

React Example

#4690 Bundle Size — 237.06KiB (0%).

f826198(current) vs 1a32dd8 main#4688(baseline)

Bundle metrics  no changes
                 Current
#4690
     Baseline
#4688
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 158 158
No change  Duplicate Modules 64 64
No change  Duplicate Code 45.83% 45.83%
No change  Packages 2 2
No change  Duplicate Packages 0 0
Bundle size by type  no changes
                 Current
#4690
     Baseline
#4688
No change  IMG 145.76KiB 145.76KiB
No change  Other 91.3KiB 91.3KiB

Bundle analysis reportBranch hzy:p/hzy/bench_2Project dashboard


Generated by RelativeCIDocumentationReport issue

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Aug 28, 2025

CodSpeed Performance Report

Merging #1623 will degrade performances by 11.62%

Comparing hzy:p/hzy/bench_2 (f826198) with main (1a32dd8)

🎉 Hooray! codspeed-node just leveled up to 4.0.1!

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

❌ 2 regressions
✅ 17 untouched benchmarks
🆕 1 new benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
🆕 more faster fib(20) N/A 16.2 µs N/A
002-hello-reactLynx-commitChanges 86.3 µs 97.7 µs -11.62%
basic-performance-nest-level-100 6.2 ms 7 ms -11.61%

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: 0

🧹 Nitpick comments (2)
benchmark/react/cases/001-fib/index.ts (2)

27-37: Rename fib3 and align base-case check.

Use a descriptive name and match the base-case style used in fib/fib2.

-  function fib3(n: number): number {
-    if (n === 0) return 0;
-    if (n === 1) return 1;
+  function fibIterative(n: number): number {
+    if (n <= 1) return n;
     let prev = 0, curr = 1;
     for (let i = 2; i <= n; i++) {
       const next = prev + curr;
       prev = curr;
       curr = next;
     }
     return curr;
   }

48-50: Use clear, stable task name and update call.

Avoid “more faster”; prefer algorithmic naming for result stability.

-    .add(`${__REPO_FILEPATH__}::more faster fib(20)`, () => {
-      fib3(20);
+    .add(`${__REPO_FILEPATH__}::iterative fib(20)`, () => {
+      fibIterative(20);
     })

Note: Consider renaming the other tasks to memoized fib(20) and recursive fib(20) for consistency.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 19f2452 and f826198.

📒 Files selected for processing (2)
  • benchmark/react/cases/001-fib/index.ts (2 hunks)
  • packages/lynx/benchx_cli/scripts/build.mjs (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/lynx/benchx_cli/scripts/build.mjs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: build / Build (Windows)
  • GitHub Check: build / Build (Ubuntu)
  • GitHub Check: test-rust / Test (Ubuntu)
  • GitHub Check: CodeQL Analyze (actions)
  • GitHub Check: CodeQL Analyze (javascript-typescript)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants