Skip to content

feat: add a trace for renderMainThread #2398

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

Closed
HuJean wants to merge 1 commit into from
Closed

feat: add a trace for renderMainThread #2398

Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions .changeset/itchy-dogs-search.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---

---
8 changes: 7 additions & 1 deletion packages/react/runtime/src/lifecycle/render.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@ import { __root } from '../root.js';
import { SnapshotInstance } from '../snapshot/snapshot.js';

function renderMainThread(): void {
if (typeof __PROFILE__ !== 'undefined' && __PROFILE__) {
profileStart('ReactLynx::renderMainThread');
}
Comment on lines +18 to +20
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 30, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Expand the existing renderMainThread trace instead of adding a second one.

Lines 23-33 already manage a ReactLynx::renderMainThread span. Adding another start here and another end on Line 58 nests the same trace name, and the new outer end is skipped if the test-only opcode remap or renderOpcodesInto() throws. Since profileEnd just forwards to the native profiler, that can leave profiling state unbalanced and pollute later measurements. If the intent is to cover the whole function, widen the current trace to a single outer try/finally and drop the inner duplicate.

Also applies to: 57-59

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/lifecycle/render.ts` around lines 18 - 20, The
duplicate profiling span should be removed and the existing
ReactLynx::renderMainThread trace (created via profileStart/profileEnd) should
be expanded to cover the entire renderMainThread function; move the single
profileStart to the outermost entry of renderMainThread and wrap the function
body in a try/finally that always calls profileEnd, removing the inner
profileStart/profileEnd pair that currently encloses renderOpcodesInto() and the
test-only opcode remap so the trace is not nested or left unbalanced if remap or
renderOpcodesInto() throws.

let opcodes;
try {
if (typeof __PROFILE__ !== 'undefined' && __PROFILE__) {
profileStart('ReactLynx::renderMainThread');
profileStart('ReactLynx::renderToString');
}
opcodes = renderToString(__root.__jsx, undefined);
} catch (e) {
Expand Down Expand Up @@ -51,6 +54,9 @@ function renderMainThread(): void {
if (typeof __PROFILE__ !== 'undefined' && __PROFILE__) {
profileEnd();
}
if (typeof __PROFILE__ !== 'undefined' && __PROFILE__) {
profileEnd();
}
}

export { renderMainThread };
Loading