forked from facebook/react
-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
compiler: merge reactive scopes across const StoreLocal (facebook#29095)
@jbonta nerd-sniped me into making this optimization during conference prep, posting this as a PR now that keynote is over. Consider these two cases: ```javascript export default function MyApp1({ count }) { const cb = () => count; return <div onclick={cb}>Hello World</div>; } export default function MyApp2({ count }) { return <div onclick={() => count}>Hello World</div>; } ``` Previously, the former would create two reactive scopes (one for `cb`, one for the div) while the latter would only have a single scope for the `div` and its inline callback. The reason we created separate scopes before is that there's a `StoreLocal 'cb' = t0` instruction in-between, and i had conservatively implemented the merging pass to not allow intervening StoreLocal instructions. The observation is that intervening StoreLocals are fine _if_ the assigned variable's last usage is the next scope. We already have a check that the intervening lvalues are last-used at/before the next scope, so it's trivial to extend this to support StoreLocal. Note that we already don't merge scopes if there are intervening terminals, so we don't have to worry about things like conditional StoreLocal, conditional access of the resulting value, etc.
- Loading branch information
1 parent
7a149aa
commit 5ab5471
Showing
22 changed files
with
304 additions
and
211 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
70 changes: 70 additions & 0 deletions
70
...ixtures/compiler/dont-merge-overlapping-scopes-store-const-used-later.expect.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
|
|
||
| ## Input | ||
|
|
||
| ```javascript | ||
| import { Stringify, makeObject_Primitives } from "shared-runtime"; | ||
|
|
||
| function Component(props) { | ||
| const array = [props.count]; | ||
| const x = makeObject_Primitives(); | ||
| const element = <div>{array}</div>; | ||
| console.log(x); | ||
| return <div>{element}</div>; | ||
| } | ||
|
|
||
| export const FIXTURE_ENTRYPOINT = { | ||
| fn: Component, | ||
| params: [{ count: 42 }], | ||
| }; | ||
|
|
||
| ``` | ||
|
|
||
| ## Code | ||
|
|
||
| ```javascript | ||
| import { c as _c } from "react/compiler-runtime"; | ||
| import { Stringify, makeObject_Primitives } from "shared-runtime"; | ||
|
|
||
| function Component(props) { | ||
| const $ = _c(6); | ||
| let t0; | ||
| if ($[0] !== props.count) { | ||
| t0 = [props.count]; | ||
| $[0] = props.count; | ||
| $[1] = t0; | ||
| } else { | ||
| t0 = $[1]; | ||
| } | ||
| const array = t0; | ||
| const x = makeObject_Primitives(); | ||
| let t1; | ||
| if ($[2] !== array) { | ||
| t1 = <div>{array}</div>; | ||
| $[2] = array; | ||
| $[3] = t1; | ||
| } else { | ||
| t1 = $[3]; | ||
| } | ||
| const element = t1; | ||
| console.log(x); | ||
| let t2; | ||
| if ($[4] !== element) { | ||
| t2 = <div>{element}</div>; | ||
| $[4] = element; | ||
| $[5] = t2; | ||
| } else { | ||
| t2 = $[5]; | ||
| } | ||
| return t2; | ||
| } | ||
|
|
||
| export const FIXTURE_ENTRYPOINT = { | ||
| fn: Component, | ||
| params: [{ count: 42 }], | ||
| }; | ||
|
|
||
| ``` | ||
| ### Eval output | ||
| (kind: ok) <div><div>42</div></div> | ||
| logs: [{ a: 0, b: 'value1', c: true }] |
14 changes: 14 additions & 0 deletions
14
...r/src/__tests__/fixtures/compiler/dont-merge-overlapping-scopes-store-const-used-later.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import { Stringify, makeObject_Primitives } from "shared-runtime"; | ||
|
|
||
| function Component(props) { | ||
| const array = [props.count]; | ||
| const x = makeObject_Primitives(); | ||
| const element = <div>{array}</div>; | ||
| console.log(x); | ||
| return <div>{element}</div>; | ||
| } | ||
|
|
||
| export const FIXTURE_ENTRYPOINT = { | ||
| fn: Component, | ||
| params: [{ count: 42 }], | ||
| }; |
Oops, something went wrong.