From ca18e1de111cd09cfbea0d5cb7fedff20592f3e3 Mon Sep 17 00:00:00 2001 From: Mike Vitousek Date: Wed, 29 May 2024 12:30:00 -0700 Subject: [PATCH] [compiler] Option to always take the non-memo branch Summary: This adds a debugging mode to the compiler that simply adds a `|| true` to the guard on all memoization blocks, which results in the generated code never using memoized values and always recomputing them. This is designed as a validation tool for the compiler's correctness--every program *should* behave exactly the same with this option enabled as it would with it disabled, and so any difference in behavior should be investigated as either a compiler bug or a pipeline issue. (We add `|| true` rather than dropping the conditional block entirely because we still want to exercise the guard tests, in case the guards themselves are the source of an error, like reading a property from undefined in a guard.) [ghstack-poisoned] --- .../src/Entrypoint/Pipeline.ts | 5 +- .../src/HIR/Environment.ts | 9 +++ .../ReactiveScopes/CodegenReactiveFunction.ts | 6 ++ .../useMemo-simple-preserved-nomemo.expect.md | 66 +++++++++++++++++++ .../useMemo-simple-preserved-nomemo.js | 13 ++++ compiler/packages/snap/src/compiler.ts | 5 ++ 6 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-simple-preserved-nomemo.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-simple-preserved-nomemo.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 258cb9199e6..303397f6116 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -148,7 +148,10 @@ function* runWithEnvironment( validateContextVariableLValues(hir); validateUseMemo(hir); - if (!env.config.enablePreserveExistingManualUseMemo) { + if ( + !env.config.enablePreserveExistingManualUseMemo && + !env.config.disableMemoizationForDebugging + ) { dropManualMemoization(hir); yield log({ kind: "hir", name: "DropManualMemoization", value: hir }); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 7375c35c765..6809742d80a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -350,6 +350,15 @@ const EnvironmentConfigSchema = z.object({ */ enableTreatFunctionDepsAsConditional: z.boolean().default(false), + /** + * When true, always act as though the dependencies of a memoized value + * have changed. This makes the compiler not actually perform any optimizations, + * but is useful for debugging. Implicitly also sets + * @enablePreserveExistingManualUseMemo, because otherwise memoization in the + * original source will be disabled as well. + */ + disableMemoizationForDebugging: z.boolean().default(false), + /** * The react native re-animated library uses custom Babel transforms that * requires the calls to library API remain unmodified. diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 075fb987922..16ee4fed563 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -614,6 +614,12 @@ function codegenReactiveScope( [t.stringLiteral(MEMO_CACHE_SENTINEL)] ) ); + } else if (cx.env.config.disableMemoizationForDebugging) { + testCondition = t.logicalExpression( + "||", + testCondition, + t.booleanLiteral(true) + ); } let computationBlock = codegenBlock(cx, block); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-simple-preserved-nomemo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-simple-preserved-nomemo.expect.md new file mode 100644 index 00000000000..f9a0672e320 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-simple-preserved-nomemo.expect.md @@ -0,0 +1,66 @@ + +## Input + +```javascript +// @disableMemoizationForDebugging +import { useMemo } from "react"; + +function Component({ a }) { + let x = useMemo(() => [a], []); + return
{x}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: 42 }], + isComponent: true, +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @disableMemoizationForDebugging +import { useMemo } from "react"; + +function Component(t0) { + const $ = _c(5); + const { a } = t0; + let t1; + if ($[0] !== a || true) { + t1 = () => [a]; + $[0] = a; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t2 = []; + $[2] = t2; + } else { + t2 = $[2]; + } + const x = useMemo(t1, t2); + let t3; + if ($[3] !== x || true) { + t3 =
{x}
; + $[3] = x; + $[4] = t3; + } else { + t3 = $[4]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: 42 }], + isComponent: true, +}; + +``` + +### Eval output +(kind: ok)
42
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-simple-preserved-nomemo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-simple-preserved-nomemo.js new file mode 100644 index 00000000000..b68649d9283 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-simple-preserved-nomemo.js @@ -0,0 +1,13 @@ +// @disableMemoizationForDebugging +import { useMemo } from "react"; + +function Component({ a }) { + let x = useMemo(() => [a], []); + return
{x}
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: 42 }], + isComponent: true, +}; diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index c949fa7fbe1..29c85a053dd 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -44,6 +44,7 @@ function makePluginOptions( // TODO(@mofeiZ) rewrite snap fixtures to @validatePreserveExistingMemo:false let validatePreserveExistingMemoizationGuarantees = false; let enablePreserveExistingManualUseMemo = false; + let disableMemoizationForDebugging = false; if (firstLine.indexOf("@compilationMode(annotation)") !== -1) { assert( @@ -124,6 +125,9 @@ function makePluginOptions( if (firstLine.includes("@enablePreserveExistingManualUseMemo")) { enablePreserveExistingManualUseMemo = true; } + if (firstLine.includes("@disableMemoizationForDebugging")) { + disableMemoizationForDebugging = true; + } const hookPatternMatch = /@hookPattern:"([^"]+)"/.exec(firstLine); if ( hookPatternMatch && @@ -178,6 +182,7 @@ function makePluginOptions( hookPattern, validatePreserveExistingMemoizationGuarantees, enablePreserveExistingManualUseMemo, + disableMemoizationForDebugging, }, compilationMode, logger: null,