Skip to content

[Fizz] Share code between inline and external runtime #33066

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.

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
merged 2 commits into from
May 1, 2025

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Apr 30, 2025

Stacked on #33065.

The runtime is about to be a lot more complicated so we need to start sharing some more code.

The problem with sharing code is that we want the inline runtime to as much as possible be isolated in its scope using only a few global variables to refer across runtimes.

A problem with Closure Compiler is that it refuses to inline functions if they have closures inside of them. Which makes sense because of how VMs work it can cause memory leaks. However, in our cases this doesn't matter and code size matters more. So we can't use many clever tricks.

So this just favors writing the source in the inline form. Then we add an extra compiler pass to turn those global variables into local variables in the external runtime.

@sebmarkbage sebmarkbage requested review from gnoff and mofeiZ April 30, 2025 17:12
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Apr 30, 2025
@react-sizebot
Copy link

react-sizebot commented Apr 30, 2025

Comparing: 71797c8...4ecb525

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 527.81 kB 527.81 kB = 93.08 kB 93.08 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 633.44 kB 633.44 kB = 111.27 kB 111.27 kB
facebook-www/ReactDOM-prod.classic.js = 671.22 kB 671.22 kB = 117.71 kB 117.71 kB
facebook-www/ReactDOM-prod.modern.js = 661.50 kB 661.50 kB = 116.15 kB 116.15 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 4ecb525

@sebmarkbage sebmarkbage merged commit bb57fa7 into facebook:main May 1, 2025
239 checks passed
github-actions bot pushed a commit that referenced this pull request May 1, 2025
Stacked on #33065.

The runtime is about to be a lot more complicated so we need to start
sharing some more code.

The problem with sharing code is that we want the inline runtime to as
much as possible be isolated in its scope using only a few global
variables to refer across runtimes.

A problem with Closure Compiler is that it refuses to inline functions
if they have closures inside of them. Which makes sense because of how
VMs work it can cause memory leaks. However, in our cases this doesn't
matter and code size matters more. So we can't use many clever tricks.

So this just favors writing the source in the inline form. Then we add
an extra compiler pass to turn those global variables into local
variables in the external runtime.

DiffTrain build for [bb57fa7](bb57fa7)
github-actions bot pushed a commit to code/lib-react that referenced this pull request May 1, 2025
Stacked on facebook#33065.

The runtime is about to be a lot more complicated so we need to start
sharing some more code.

The problem with sharing code is that we want the inline runtime to as
much as possible be isolated in its scope using only a few global
variables to refer across runtimes.

A problem with Closure Compiler is that it refuses to inline functions
if they have closures inside of them. Which makes sense because of how
VMs work it can cause memory leaks. However, in our cases this doesn't
matter and code size matters more. So we can't use many clever tricks.

So this just favors writing the source in the inline form. Then we add
an extra compiler pass to turn those global variables into local
variables in the external runtime.

DiffTrain build for [bb57fa7](facebook@bb57fa7)
sebmarkbage added a commit that referenced this pull request May 1, 2025
Stacked on #33066 and #33068.

Currently we're passing `errorDigest` to `completeBoundary` if there is
a client side error (only CSS loading atm). This only exists because of
`completeBoundaryWithStyles`. Normally if there's a server-side error
we'd emit the `clientRenderBoundary` instruction instead. This adds
unnecessary code to the common case where all styles are in the head.
This is about to get worse with batching because client render shouldn't
be throttled but complete should be.

The first commit moves the client render logic inline into
`completeBoundaryWithStyles` so we only pay for it when styles are used.

However, the approach I went with in the second commit is to reuse the
`$RX` instruction instead (`clientRenderBoundary`). That way if you have
both it ends up being amortized. However, it does mean we have to emit
the `$RX` (along with the `$RC` helper if any
`completeBoundaryWithStyles` instruction is needed.
github-actions bot pushed a commit that referenced this pull request May 1, 2025
Stacked on #33066 and #33068.

Currently we're passing `errorDigest` to `completeBoundary` if there is
a client side error (only CSS loading atm). This only exists because of
`completeBoundaryWithStyles`. Normally if there's a server-side error
we'd emit the `clientRenderBoundary` instruction instead. This adds
unnecessary code to the common case where all styles are in the head.
This is about to get worse with batching because client render shouldn't
be throttled but complete should be.

The first commit moves the client render logic inline into
`completeBoundaryWithStyles` so we only pay for it when styles are used.

However, the approach I went with in the second commit is to reuse the
`$RX` instruction instead (`clientRenderBoundary`). That way if you have
both it ends up being amortized. However, it does mean we have to emit
the `$RX` (along with the `$RC` helper if any
`completeBoundaryWithStyles` instruction is needed.

DiffTrain build for [ee077b6](ee077b6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants