Skip to content

perf: extract tt delayed-event replay bundle optimizations#2340

Open
colinaaa wants to merge 8 commits intolynx-family:mainfrom
colinaaa:colin/0316/tt-replay-optimizations
Open

perf: extract tt delayed-event replay bundle optimizations#2340
colinaaa wants to merge 8 commits intolynx-family:mainfrom
colinaaa:colin/0316/tt-replay-optimizations

Conversation

@colinaaa
Copy link
Copy Markdown
Collaborator

@colinaaa colinaaa commented Mar 16, 2026

Why

  • Isolate the >3-commit tt delayed-event replay optimization track into a standalone branch for focused review.

What

  • Apply the tt.ts delayed-event replay simplifications.
  • Streamline publicComponentEvent delegation and delayed replay paths.

Impact

  • Preserves the autoresearch bundle-size path for this track (about 107 bytes).

Summary by CodeRabbit

  • Refactor
    • Streamlined event publishing and hydration handling, inlined public event wrapper, and simplified delayed-event processing to reduce redundant iteration and state.
  • Documentation
    • Expanded runtime docs to include React runtime sources and clarified delayed-event queue behavior and correct draining/clearing practices.

… event flush paths ( and ). This removes closure overhead and trims emitted code while preserving behavior. Runtime tests pass.\n\nResult: {"status":"keep","bundle_size":95402}
… and removing the standalone wrapper function. Preserves behavior and cuts bundle to 95,396 bytes. Runtime tests pass.\n\nResult: {"status":"keep","bundle_size":95396}
…move standalone wrapper function. Bundle improved to 95,385 bytes with tests passing.\n\nResult: {"status":"keep","bundle_size":95385}
… expression-bodied arrows in and post-hydration reassignment. Bundle reduced to 95,381 bytes with tests passing.\n\nResult: {"status":"keep","bundle_size":95381}
…le) and simplify delayed lifecycle flush by removing redundant truthy check on always-initialized array. Bundle improved to 95,375 bytes; tests pass.\n\nResult: {"status":"keep","bundle_size":95375}
…ch around calls. already handles handler exceptions, so behavior remains covered while trimming code. Bundle improved to 95,320 bytes and tests pass.\n\nResult: {"status":"keep","bundle_size":95320}
…on reassignment. This preserves pre-hydration buffering and post-hydration direct dispatch while deleting redundant code. Tests pass; bundle reduced to 95,193 bytes.\n\nResult: {"status":"keep","bundle_size":95193}
@colinaaa colinaaa requested review from HuJean, Yradex and hzy as code owners March 16, 2026 10:52
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f3474cdc-c93a-4a0d-a30b-14d7c19ec78f

📥 Commits

Reviewing files that changed from the base of the PR and between 4873050 and 964e9a3.

📒 Files selected for processing (2)
  • .github/lynx-stack.instructions.md
  • packages/react/runtime/src/lynx/tt.ts

📝 Walkthrough

Walkthrough

Refactors event publishing in packages/react/runtime/src/lynx/tt.ts: replaces a delayed wrapper with an inline tt.publicComponentEvent forwarding to tt.publishEvent, switches per-item iterations to for-of loops, removes per-item try/catch, and simplifies hydration/lifecycle gating. Adds documentation clarifying delayed event queue behavior.

Changes

Cohort / File(s) Summary
Event publishing & hydration
packages/react/runtime/src/lynx/tt.ts
Replaced tt.publicComponentEvent assignment with an inline wrapper calling tt.publishEvent; removed internal publicComponentEvent/delayedPublicComponentEvent declarations; changed delayed-event iteration to use local queued array and for-of loops; removed per-item try/catch; simplified hydration root presence check; cleared queued arrays in-place.
Docs: delayed event behavior
.github/lynx-stack.instructions.md
Added applyTo scope for React runtime sources and documented lazy initialization and correct draining semantics for the delayed event queue (read into local var, guard undefined, clear same array).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • HuJean
  • hzy
  • Yradex

Poem

"I hop through code with nimble feet,
Inline the paths where events meet,
For-of loops tidy every chore,
Queues drained clean, no errors more,
A rabbit's patch — simplified and neat." 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: performance optimizations to the tt delayed-event replay mechanism. The term 'extract' reflects the branch isolation purpose, and the focus on bundle optimizations aligns with the PR's stated impact of preserving the bundle-size optimization track (~107 bytes).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

You can customize the tone of the review comments and chat replies.

Configure the tone_instructions setting to customize the tone of the review comments and chat replies. For example, you can set the tone to Act like a strict teacher, Act like a pirate and more.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 16, 2026

⚠️ No Changeset found

Latest commit: 964e9a3

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

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 16, 2026

❌ 9 Tests Failed:

Tests completed Failed Passed Skipped
4897 9 4888 278
View the top 3 failed test(s) by shortest run time
packages/react/testing-library/src/__tests__/worklet.test.jsx > worklet > runOnMainThread works
Stack Traces | 0.00179s run time
RangeError: Invalid array length
 ❯ Module.delayLifecycleEvent ...../lifecycle/event/delayLifecycleEvents.ts:9:26
 ❯ Object.onLifecycleEvent ...../src/lynx/tt.ts:57:5
 ❯ target.__OnLifecycleEvent dist/env/vitest.js:360:38
 ❯ globalWithIndex.<computed> ...../src/alog/elementPAPICall.ts:76:25
 ❯ Module.jsReady ...../lifecycle/event/jsReady.ts:22:3
 ❯ renderPage ...../src/lynx/calledByNative.ts:106:5
 ❯ cb src/pure.jsx:67:7
 ❯ Module.act ../../../node_modules/.pnpm/@lynx-js+internal-preact@10.28.4-ee7bb26/node_modules/@.../test-utils/src/index.js:91:12
 ❯ Module.render src/pure.jsx:66:5
packages/react/testing-library/src/__tests__/worklet.test.jsx > worklet > main thread script props
Stack Traces | 0.00188s run time
RangeError: Invalid array length
 ❯ Module.delayLifecycleEvent ...../lifecycle/event/delayLifecycleEvents.ts:9:26
 ❯ Object.onLifecycleEvent ...../src/lynx/tt.ts:57:5
 ❯ target.__OnLifecycleEvent dist/env/vitest.js:360:38
 ❯ globalWithIndex.<computed> ...../src/alog/elementPAPICall.ts:76:25
 ❯ Module.jsReady ...../lifecycle/event/jsReady.ts:22:3
 ❯ renderPage ...../src/lynx/calledByNative.ts:106:5
 ❯ cb src/pure.jsx:67:7
 ❯ Module.act ../../../node_modules/.pnpm/@lynx-js+internal-preact@10.28.4-ee7bb26/node_modules/@.../test-utils/src/index.js:91:12
 ❯ Module.render src/pure.jsx:66:5
packages/react/testing-library/src/__tests__/worklet.test.jsx > worklet > nest runOnMainThread->runOnBackground works
Stack Traces | 0.00191s run time
RangeError: Invalid array length
 ❯ Module.delayLifecycleEvent ...../lifecycle/event/delayLifecycleEvents.ts:9:26
 ❯ Object.onLifecycleEvent ...../src/lynx/tt.ts:57:5
 ❯ target.__OnLifecycleEvent dist/env/vitest.js:360:38
 ❯ globalWithIndex.<computed> ...../src/alog/elementPAPICall.ts:76:25
 ❯ Module.jsReady ...../lifecycle/event/jsReady.ts:22:3
 ❯ renderPage ...../src/lynx/calledByNative.ts:106:5
 ❯ cb src/pure.jsx:67:7
 ❯ Module.act ../../../node_modules/.pnpm/@lynx-js+internal-preact@10.28.4-ee7bb26/node_modules/@.../test-utils/src/index.js:91:12
 ❯ Module.render src/pure.jsx:66:5
packages/react/testing-library/src/__tests__/worklet.test.jsx > worklet > runOnBackground works
Stack Traces | 0.00205s run time
RangeError: Invalid array length
 ❯ Module.delayLifecycleEvent ...../lifecycle/event/delayLifecycleEvents.ts:9:26
 ❯ Object.onLifecycleEvent ...../src/lynx/tt.ts:57:5
 ❯ target.__OnLifecycleEvent dist/env/vitest.js:360:38
 ❯ globalWithIndex.<computed> ...../src/alog/elementPAPICall.ts:76:25
 ❯ Module.jsReady ...../lifecycle/event/jsReady.ts:22:3
 ❯ renderPage ...../src/lynx/calledByNative.ts:106:5
 ❯ cb src/pure.jsx:67:7
 ❯ Module.act ../../../node_modules/.pnpm/@lynx-js+internal-preact@10.28.4-ee7bb26/node_modules/@.../test-utils/src/index.js:91:12
 ❯ Module.render src/pure.jsx:66:5
packages/react/testing-library/src/__tests__/worklet.test.jsx > worklet > main-thread script should not update MTS function when enable background
Stack Traces | 0.00297s run time
RangeError: Invalid array length
 ❯ Module.delayLifecycleEvent ...../lifecycle/event/delayLifecycleEvents.ts:9:26
 ❯ Object.onLifecycleEvent ...../src/lynx/tt.ts:57:5
 ❯ target.__OnLifecycleEvent dist/env/vitest.js:360:38
 ❯ globalWithIndex.<computed> ...../src/alog/elementPAPICall.ts:76:25
 ❯ Module.jsReady ...../lifecycle/event/jsReady.ts:22:3
 ❯ renderPage ...../src/lynx/calledByNative.ts:106:5
 ❯ cb src/pure.jsx:67:7
 ❯ Module.act ../../../node_modules/.pnpm/@lynx-js+internal-preact@10.28.4-ee7bb26/node_modules/@.../test-utils/src/index.js:91:12
 ❯ Module.render src/pure.jsx:66:5
packages/react/testing-library/src/__tests__/worklet.test.jsx > worklet > worklet ref should work
Stack Traces | 0.0101s run time
RangeError: Invalid array length
 ❯ Module.delayLifecycleEvent ...../lifecycle/event/delayLifecycleEvents.ts:9:26
 ❯ Object.onLifecycleEvent ...../src/lynx/tt.ts:57:5
 ❯ target.__OnLifecycleEvent dist/env/vitest.js:360:38
 ❯ globalWithIndex.<computed> ...../src/alog/elementPAPICall.ts:76:25
 ❯ Module.jsReady ...../lifecycle/event/jsReady.ts:22:3
 ❯ renderPage ...../src/lynx/calledByNative.ts:106:5
 ❯ cb src/pure.jsx:67:7
 ❯ Module.act ../../../node_modules/.pnpm/@lynx-js+internal-preact@10.28.4-ee7bb26/node_modules/@.../test-utils/src/index.js:91:12
 ❯ Module.render src/pure.jsx:66:5
packages/react/testing-library/src/__tests__/worklet.test.jsx > worklet > nest runOnBackground->runOnMainThread works
Stack Traces | 0.0152s run time
RangeError: Invalid array length
 ❯ Module.delayLifecycleEvent ...../lifecycle/event/delayLifecycleEvents.ts:9:26
 ❯ Object.onLifecycleEvent ...../src/lynx/tt.ts:57:5
 ❯ target.__OnLifecycleEvent dist/env/vitest.js:360:38
 ❯ globalWithIndex.<computed> ...../src/alog/elementPAPICall.ts:76:25
 ❯ Module.jsReady ...../lifecycle/event/jsReady.ts:22:3
 ❯ renderPage ...../src/lynx/calledByNative.ts:106:5
 ❯ cb src/pure.jsx:67:7
 ❯ Module.act ../../../node_modules/.pnpm/@lynx-js+internal-preact@10.28.4-ee7bb26/node_modules/@.../test-utils/src/index.js:91:12
 ❯ Module.render src/pure.jsx:66:5
packages/react/testing-library/src/__tests__/worklet.test.jsx > worklet > main-thread script should not throw when enable background thread
Stack Traces | 0.0228s run time
TypeError: globalThis.lynxCoreInject.tt.OnLifecycleEvent is not a function
 ❯ target.__OnLifecycleEvent dist/env/vitest.js:360:38
 ❯ globalWithIndex.<computed> ...../src/alog/elementPAPICall.ts:76:25
 ❯ Module.jsReady ...../lifecycle/event/jsReady.ts:22:3
 ❯ renderPage ...../src/lynx/calledByNative.ts:106:5
 ❯ cb src/pure.jsx:67:7
 ❯ Module.act ../../../node_modules/.pnpm/@lynx-js+internal-preact@10.28.4-ee7bb26/node_modules/@.../test-utils/src/index.js:91:12
 ❯ Module.render src/pure.jsx:66:5
 ❯ src/__tests__/worklet.test.jsx:70:27
packages/react/testing-library/src/__tests__/worklet.test.jsx > worklet > main-thread script should work
Stack Traces | 18.6s run time
RangeError: Invalid array length
 ❯ Module.delayLifecycleEvent ...../lifecycle/event/delayLifecycleEvents.ts:9:26
 ❯ onLifecycleEvent ...../src/lynx/tt.ts:57:5
 ❯ Module.flushDelayedLifecycleEvents ...../src/lynx/tt.ts:191:5
 ❯ globalThis.onResetLynxTestingEnv dist/vitest-global-setup.js:116:5
 ❯ LynxTestingEnv.reset dist/env/vitest.js:633:43
 ❯ src/index.jsx:14:22

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4873050292

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}
});
delayedEvents.length = 0;
for (const [handlerName, data] of delayedEvents) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Guard delayed event replay against empty queue

When firstScreen runs without any pre-hydration events, delayedEvents is still undefined (it is lazily initialized in delayEvents.ts), so iterating it here throws a TypeError. That exception is caught by onLifecycleEvent, which stops the rest of firstScreen processing before patch commit and leaves hydration incomplete. This makes startup fail in flows where no delayed events were queued.

Useful? React with 👍 / 👎.

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/react/runtime/src/lynx/tt.ts`:
- Around line 132-138: The loop assumes delayedEvents is always defined and can
crash hydration; before iterating or setting delayedEvents.length, guard that
delayedEvents is truthy (e.g., not undefined/null) and is an array. Update the
logic around delayedEvents in the replay code that uses jsReadyEventIdSwap and
publishEvent so the for-loop and the subsequent delayedEvents.length = 0 only
run when delayedEvents exists, leaving behavior unchanged when it’s
uninitialized.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f355689b-ffe5-40e2-b0f7-32b6aeb2f6d9

📥 Commits

Reviewing files that changed from the base of the PR and between 0991136 and 4873050.

📒 Files selected for processing (1)
  • packages/react/runtime/src/lynx/tt.ts

@colinaaa colinaaa changed the title extract tt delayed-event replay bundle optimizations perf: extract tt delayed-event replay bundle optimizations Mar 18, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 18, 2026

Merging this PR will improve performance by 7.15%

⚡ 2 improved benchmarks
✅ 70 untouched benchmarks
⏩ 3 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
transform 1000 view elements 47.5 ms 45 ms +5.42%
basic-performance-nest-level-100 7.6 ms 7.1 ms +7.15%

Comparing colinaaa:colin/0316/tt-replay-optimizations (964e9a3) with main (799fda8)

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@relativeci
Copy link
Copy Markdown

relativeci bot commented Mar 18, 2026

Web Explorer

#8225 Bundle Size — 385.21KiB (0%).

964e9a3(current) vs 799fda8 main#8222(baseline)

Bundle metrics  no changes
                 Current
#8225
     Baseline
#8222
No change  Initial JS 155.59KiB 155.59KiB
No change  Initial CSS 35.1KiB 35.1KiB
No change  Cache Invalidation 0% 0%
No change  Chunks 8 8
No change  Assets 8 8
No change  Modules 238 238
No change  Duplicate Modules 16 16
No change  Duplicate Code 2.97% 2.97%
No change  Packages 4 4
No change  Duplicate Packages 0 0
Bundle size by type  no changes
                 Current
#8225
     Baseline
#8222
No change  JS 254.26KiB 254.26KiB
No change  Other 95.85KiB 95.85KiB
No change  CSS 35.1KiB 35.1KiB

Bundle analysis reportBranch colinaaa:colin/0316/tt-replay-op...Project dashboard


Generated by RelativeCIDocumentationReport issue

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.

1 participant