Skip to content
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

[compiler] Debug tool to emit change detection code rather than memoization #29656

Merged
merged 7 commits into from
May 31, 2024

Conversation

mvitousek
Copy link
Contributor

@mvitousek mvitousek commented May 29, 2024

Stack from ghstack (oldest at bottom):

Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent.

This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise?

This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will still perform the computation, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame.

This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation.

…zation

Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent.

This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise?

This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will *still perform the computation*, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame.

This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation.

[ghstack-poisoned]
Copy link

vercel bot commented May 29, 2024

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 May 31, 2024 9:10pm

@react-sizebot
Copy link

react-sizebot commented May 29, 2024

Comparing: 63d673c...366c1a7

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.66 kB 6.66 kB = 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 496.39 kB 496.39 kB = 88.84 kB 88.84 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 501.21 kB 501.21 kB = 89.54 kB 89.54 kB
facebook-www/ReactDOM-prod.classic.js = 593.88 kB 593.88 kB = 104.46 kB 104.46 kB
facebook-www/ReactDOM-prod.modern.js = 570.26 kB 570.26 kB = 100.87 kB 100.87 kB
test_utils/ReactAllWarnings.js Deleted 63.65 kB 0.00 kB Deleted 15.90 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 63.65 kB 0.00 kB Deleted 15.90 kB 0.00 kB

Generated by 🚫 dangerJS against 366c1a7

… than memoization"

Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent.

This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise?

This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will *still perform the computation*, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame.

This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation.

[ghstack-poisoned]
Copy link
Contributor

@josephsavona josephsavona left a comment

Choose a reason for hiding this comment

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

nice. this is mutually exclusive with the option to always emit || true in the if check, right? let's enforce that if so.

@@ -128,6 +129,12 @@ function makePluginOptions(
if (firstLine.includes("@disableMemoizationForDebugging")) {
disableMemoizationForDebugging = true;
}
if (firstLine.includes("@enableChangeDetectionForDebugging")) {
enableChangeDetectionForDebugging = {
source: "react-compiler-runtime",
Copy link
Member

Choose a reason for hiding this comment

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

fyi you might already know this but we don't currently publish this package on npm, so this flag will only work internally for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I wanted to include the runtime code as basically an example of/documentation for what the change detection code would look like. I've been updating it internally separately

… than memoization"

Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent.

This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise?

This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will *still perform the computation*, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame.

This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation.

[ghstack-poisoned]
… than memoization"

Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent.

This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise?

This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will *still perform the computation*, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame.

This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation.

[ghstack-poisoned]
Copy link
Contributor

@josephsavona josephsavona left a comment

Choose a reason for hiding this comment

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

nice

… than memoization"

Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent.

This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise?

This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will *still perform the computation*, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame.

This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation.

[ghstack-poisoned]
… than memoization"

Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent.

This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise?

This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will *still perform the computation*, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame.

This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation.

[ghstack-poisoned]
… than memoization"

Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent.

This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise?

This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will *still perform the computation*, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame.

This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation.

[ghstack-poisoned]
@mvitousek mvitousek merged commit b8ed3ef into gh/mvitousek/3/base May 31, 2024
8 of 10 checks passed
mvitousek added a commit that referenced this pull request May 31, 2024
…zation

Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent.

This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise?

This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will *still perform the computation*, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame.

This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation.

ghstack-source-id: 50dad3dacfc7fef74be350431aa2ebf5e9cb0031
Pull Request resolved: #29656
@mvitousek mvitousek deleted the gh/mvitousek/3/head branch May 31, 2024 21:06
github-actions bot pushed a commit that referenced this pull request May 31, 2024
…zation

Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent.

This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise?

This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will *still perform the computation*, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame.

This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation.

ghstack-source-id: 50dad3dacfc7fef74be350431aa2ebf5e9cb0031
Pull Request resolved: #29656

DiffTrain build for commit 5c420e3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants