[compiler] Add JSX inlining optimization #30867

jackpope · 2024-09-03T21:25:51Z

This adds an InlineJsxTransform optimization pass, toggled by the enableInlineJsxTransform flag. When enabled, JSX will be transformed into React Element object literals, preventing runtime overhead during element creation.

TODO:

Add conditionals to make transform PROD-only
Make the React element symbol configurable so this works with runtimes that support react.element or react.transitional.element
Look into additional optimization to pass props spread through directly if none of the properties are mutated

vercel · 2024-09-03T21:25:54Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 18, 2024 3:31pm

jackpope · 2024-09-03T21:27:34Z

compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts

+  /**
+   * Fixup the HIR to restore RPO, ensure correct predecessors, and
+   * renumber instructions. Note that the renumbering instructions
+   * invalidates scope and identifier ranges, so we fix them in the
+   * next step.
+   */
+  reversePostorderBlocks(fn.body);
+  markPredecessors(fn.body);
+  markInstructionIds(fn.body);
+
+  /**
+   * Fix scope and identifier ranges to account for renumbered instructions
+   */
+  for (const [, block] of fn.body.blocks) {
+    const terminal = block.terminal;
+    if (terminal.kind === 'scope' || terminal.kind === 'pruned-scope') {
+      /*
+       * Scope ranges should always align to start at the 'scope' terminal
+       * and end at the first instruction of the fallthrough block
+       */
+      const fallthroughBlock = fn.body.blocks.get(terminal.fallthrough)!;
+      const firstId =
+        fallthroughBlock.instructions[0]?.id ?? fallthroughBlock.terminal.id;
+      terminal.scope.range.start = terminal.id;
+      terminal.scope.range.end = firstId;
+    }
+  }


I stole this from BuildReactiveScopeTerminalsHIR.ts. Should this be a shared utility?

I think we should extract the "fix scope and identifier ranges..." for loop into a shared utility and then we can call it where necessary. There are some passes that need to fix up scopes but don't need to run reversePostorderBlocks and markPredecessors, so putting all 4 of these calls into a utility doesn't make sense.

sebmarkbage · 2024-09-03T21:31:05Z

This should not be shipped in code that's compiled for npm since it is not compatible with multiple React versions.

It's also important that it doesn't ship in an RSC layer which needs a runtime to do the warm up pass.

(In addition, previous testing showed that this is not actually better than a minimal runtime in most environments due to the additional compilation cost.)

sebmarkbage · 2024-09-03T21:42:08Z

To clarify the last statement.

Most testing of inline objects have compared it against the React runtime which has overhead mainly due to legacy features like the refs and the backwards compat with defaultProps. A proper A/B test should test the performance of inlining compared to an optimized runtime function that does the same thing that the inlining would do. Hopefully in React 20+ that can just be the same thing in the plain React.jsx function assuming deprecations land.

In that comparison it was previous discovered that the compilation time for VMs like V8 that can JIT a callsite to determine which hidden class to create but figuring that out for each callsite means it has to transition through the hidden class discovery like object with $$typeof -> object with type -> object with key -> .... Which significantly slowed down initialization compared to just making it a call and then that one call getting in a JIT:ed callsite. Runtime was slightly faster but start up significantly slower.

This effect wasn't as big in JSC possibly due to faster compiler but also due to lack of other call optimizations not being as good so the relative size isn't necessarily there. That's why you might see inlining being slightly faster in JSC. (Most comparisons like Bun's inlining 1) isn't comparing against an optimized function so not a proper A/B test 2) doesn't care about start up time. So that data is not applicable here.)

For Hermes, since the compilation time is entirely offline the cost of inlining isn't as big so the runtime overhead of the extra call might make it worth it there but it comes as the cost of larger byte code which may or may not matter.

Basically I'm skeptical overall it is actually good to inline in most circumstances and it would be better to just have a single shared runtime. Given that the output is not multi-version compatible (i.e. the strategy of using React Compiler before publishing no longer works) and it's actually worse in the largest platform (Chrome)*. And it's not compatible with JSX prewarming in RSC.

*) This could use re-measurement though since it's based on stale data.

jackpope · 2024-09-03T22:29:14Z

@sebmarkbage For now this optimization is meant to be turned on for Hermes only. I benchmarked and profiled the inlining on some test apps and saw some regressions in V8 as you described. In some apps interactions were faster, but memory regressed significantly. Running with --jitless was more stable so that lines up with your observations around JIT issues.

With Hermes, inlining showed wins to interaction performance and memory, even compared to a React 20 style jsx function. With this implementation we can test in production against real apps and understand the full impact of the change and if its worth maintaining the optimization for Hermes only.

josephsavona · 2024-09-04T19:45:39Z

compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts

+    lvalue: {...symbolForPlace, effect: Effect.Read},
+    value: {
+      kind: 'PropertyLoad',
+      object: symbolInstruction.lvalue,


note that places always have to be copied, not used by reference, so {...symbolPlace} here

josephsavona · 2024-09-04T19:46:24Z

compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts

+    case 'BuiltinTag':
+      const tagPropertyPlace = createTemporaryPlace(fn.env, instr.value.loc);


always wrap cases in {}

josephsavona

This is great! A few minor changes and then we should be able to land and experiment.

josephsavona

Looks great! Just one quick thing to double-check for when there's a single child element.

josephsavona · 2024-09-16T22:36:31Z

...s/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inline-jsx-transform.expect.md

+      type: "div",
+      ref: ref,
+      key: null,
+      props: { children: [children] },


should this be an array ([children]) or just the raw children value?

josephsavona · 2024-09-18T15:44:14Z

Nice! Let's ship and iterate

This adds an `InlineJsxTransform` optimization pass, toggled by the `enableInlineJsxTransform` flag. When enabled, JSX will be transformed into React Element object literals, preventing runtime overhead during element creation. TODO: - [ ] Add conditionals to make transform PROD-only - [ ] Make the React element symbol configurable so this works with runtimes that support `react.element` or `react.transitional.element` - [ ] Look into additional optimization to pass props spread through directly if none of the properties are mutated DiffTrain build for [5dcb009](5dcb009)

facebook-github-bot added the CLA Signed label Sep 3, 2024

jackpope requested a review from josephsavona September 3, 2024 21:25

facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Sep 3, 2024

jackpope requested a review from mofeiZ September 3, 2024 21:26

jackpope commented Sep 3, 2024

View reviewed changes

vercel bot deployed to Preview September 3, 2024 21:31 View deployment

josephsavona reviewed Sep 4, 2024

View reviewed changes

josephsavona requested changes Sep 4, 2024

View reviewed changes

jackpope force-pushed the inline-jsx branch from 24c51eb to 8b69a6f Compare September 16, 2024 19:57

vercel bot deployed to Preview September 16, 2024 20:02 View deployment

Add inline jsx transform

2f90fcf

jackpope force-pushed the inline-jsx branch from 8b69a6f to 2f90fcf Compare September 16, 2024 20:37

vercel bot deployed to Preview September 16, 2024 20:39 View deployment

jackpope requested a review from josephsavona September 16, 2024 21:39

josephsavona approved these changes Sep 16, 2024

View reviewed changes

Use single child value directly

64b9395

vercel bot deployed to Preview September 18, 2024 15:31 View deployment

jackpope merged commit 5dcb009 into facebook:main Sep 18, 2024
19 checks passed

jackpope deleted the inline-jsx branch September 18, 2024 15:51

vercel-release-bot mentioned this pull request Sep 18, 2024

Upgrade React from a99d8e8d-20240916 to 5dcb0097-20240918 vercel/next.js#70225

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[compiler] Add JSX inlining optimization #30867

[compiler] Add JSX inlining optimization #30867

jackpope commented Sep 3, 2024

vercel bot commented Sep 3, 2024 •

edited

Loading

jackpope Sep 3, 2024

josephsavona Sep 4, 2024 •

edited

Loading

sebmarkbage commented Sep 3, 2024

sebmarkbage commented Sep 3, 2024 •

edited

Loading

jackpope commented Sep 3, 2024

josephsavona Sep 4, 2024

josephsavona Sep 4, 2024

josephsavona left a comment

josephsavona left a comment

josephsavona Sep 16, 2024

josephsavona commented Sep 18, 2024

		case 'BuiltinTag':
		const tagPropertyPlace = createTemporaryPlace(fn.env, instr.value.loc);

[compiler] Add JSX inlining optimization #30867

[compiler] Add JSX inlining optimization #30867

Conversation

jackpope commented Sep 3, 2024

vercel bot commented Sep 3, 2024 • edited Loading

jackpope Sep 3, 2024

Choose a reason for hiding this comment

josephsavona Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

sebmarkbage commented Sep 3, 2024

sebmarkbage commented Sep 3, 2024 • edited Loading

jackpope commented Sep 3, 2024

josephsavona Sep 4, 2024

Choose a reason for hiding this comment

josephsavona Sep 4, 2024

Choose a reason for hiding this comment

josephsavona left a comment

Choose a reason for hiding this comment

josephsavona left a comment

Choose a reason for hiding this comment

josephsavona Sep 16, 2024

Choose a reason for hiding this comment

josephsavona commented Sep 18, 2024

vercel bot commented Sep 3, 2024 •

edited

Loading

josephsavona Sep 4, 2024 •

edited

Loading

sebmarkbage commented Sep 3, 2024 •

edited

Loading