[rhir] Patch: ordering of overlapping input dependencies does not matter
--- Patch and simplify logic around merging overlapping reactive dependencies. Added `reduce-reactive-unconditional-deps` test fixtures, which tries to cover all cases of merging unconditional dependencies (to a minimal dependencies set). Please let me know if I missed any
Mofei Zhang committed
Feb 27, 2023 at 13:38 UTC
f335e7d4d97cabd934ee58acb44bbc1851c0a822
15 files changed
+357
-12
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+11
-12
@@ -189,24 +189,17 @@ class Context {
189
(currentDeclaration.scope == null ||
190
!this.#isScopeActive(currentDeclaration.scope))
191
) {
192
+ // Below logic ensures that `operand` is either added to `this.#dependencies`
193
+ // directly, or is covered by an existing dependency.
194
+
195
// Check if there is an existing dependency that describes this operand
196
for (const dep of this.#dependencies) {
197
// not the same identifier
198
if (dep.place.identifier.id !== maybeDependency.place.identifier.id) {
199
continue;
200
}
198
- const depPath = dep.path;
199
- // existing dep covers all paths
200
- if (depPath === null) {
201
- return;
202
- }
203
- const operandPath = maybeDependency.path;
204
- // existing dep is for a path, this operand covers all paths so swap them
205
- if (operandPath === null) {
206
- this.#dependencies.delete(dep);
207
- this.#dependencies.add(maybeDependency);
208
- return;
209
- }
201
+ const depPath = dep.path ?? [];
202
+ const operandPath = maybeDependency.path ?? [];
203
// both the operand and dep have paths, determine if the existing path
204
// is a subset of the new path
205
let commonPathIndex = 0;
@@ -218,7 +211,13 @@ class Context {
211
commonPathIndex++;
212
}
213
if (commonPathIndex === depPath.length) {
214
+ // existing dep is a subpath of the operand, so we don't need to
215
+ // add the operand
216
return;
217
+ } else if (commonPathIndex === operandPath.length) {
218
+ // operand is a subpath of the existing path, delete the existing
219
+ // path
220
+ this.#dependencies.delete(dep);
221
}
222
}
223
this.#dependencies.add(maybeDependency);
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-nonoverlap-descendant.expect.md
new
+44
@@ -0,0 +1,44 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// Test that we can track non-overlapping dependencies separately.
6
+// (not needed for correctness but for dependency granularity)
7
+function TestNonOverlappingDescendantTracked(props) {
8
+ let x = {};
9
+ x.a = props.a.x.y;
10
+ x.b = props.b;
11
+ x.c = props.a.c.x.y.z;
12
+ return x;
13
+}
14
+
15
+```
16
+
17
+## Code
18
+
19
+```javascript
20
+// Test that we can track non-overlapping dependencies separately.
21
+// (not needed for correctness but for dependency granularity)
22
+function TestNonOverlappingDescendantTracked(props) {
23
+ const $ = React.unstable_useMemoCache(4);
24
+ const c_0 = $[0] !== props.a.x.y;
25
+ const c_1 = $[1] !== props.b;
26
+ const c_2 = $[2] !== props.a.c.x.y.z;
27
+ let x;
28
+ if (c_0 || c_1 || c_2) {
29
+ x = {};
30
+ x.a = props.a.x.y;
31
+ x.b = props.b;
32
+ x.c = props.a.c.x.y.z;
33
+ $[0] = props.a.x.y;
34
+ $[1] = props.b;
35
+ $[2] = props.a.c.x.y.z;
36
+ $[3] = x;
37
+ } else {
38
+ x = $[3];
39
+ }
40
+ return x;
41
+}
42
+
43
+```
44
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-nonoverlap-descendant.js
new
+9
@@ -0,0 +1,9 @@
1
+// Test that we can track non-overlapping dependencies separately.
2
+// (not needed for correctness but for dependency granularity)
3
+function TestNonOverlappingDescendantTracked(props) {
4
+ let x = {};
5
+ x.a = props.a.x.y;
6
+ x.b = props.b;
7
+ x.c = props.a.c.x.y.z;
8
+ return x;
9
+}
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-nonoverlap-direct.expect.md
new
+40
@@ -0,0 +1,40 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// Test that we can track non-overlapping dependencies separately.
6
+// (not needed for correctness but for dependency granularity)
7
+function TestNonOverlappingTracked(props) {
8
+ let x = {};
9
+ x.b = props.a.b;
10
+ x.c = props.a.c;
11
+ return x;
12
+}
13
+
14
+```
15
+
16
+## Code
17
+
18
+```javascript
19
+// Test that we can track non-overlapping dependencies separately.
20
+// (not needed for correctness but for dependency granularity)
21
+function TestNonOverlappingTracked(props) {
22
+ const $ = React.unstable_useMemoCache(3);
23
+ const c_0 = $[0] !== props.a.b;
24
+ const c_1 = $[1] !== props.a.c;
25
+ let x;
26
+ if (c_0 || c_1) {
27
+ x = {};
28
+ x.b = props.a.b;
29
+ x.c = props.a.c;
30
+ $[0] = props.a.b;
31
+ $[1] = props.a.c;
32
+ $[2] = x;
33
+ } else {
34
+ x = $[2];
35
+ }
36
+ return x;
37
+}
38
+
39
+```
40
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-nonoverlap-direct.js
new
+8
@@ -0,0 +1,8 @@
1
+// Test that we can track non-overlapping dependencies separately.
2
+// (not needed for correctness but for dependency granularity)
3
+function TestNonOverlappingTracked(props) {
4
+ let x = {};
5
+ x.b = props.a.b;
6
+ x.c = props.a.c;
7
+ return x;
8
+}
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-overlap-descendant.expect.md
new
+40
@@ -0,0 +1,40 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// Test that we correctly track a subpath if the subpath itself is accessed as
6
+// a dependency
7
+function TestOverlappingDescendantTracked(props) {
8
+ let x = {};
9
+ x.b = props.a.b.c;
10
+ x.c = props.a.b.c.x.y;
11
+ x.a = props.a;
12
+ return x;
13
+}
14
+
15
+```
16
+
17
+## Code
18
+
19
+```javascript
20
+// Test that we correctly track a subpath if the subpath itself is accessed as
21
+// a dependency
22
+function TestOverlappingDescendantTracked(props) {
23
+ const $ = React.unstable_useMemoCache(2);
24
+ const c_0 = $[0] !== props.a;
25
+ let x;
26
+ if (c_0) {
27
+ x = {};
28
+ x.b = props.a.b.c;
29
+ x.c = props.a.b.c.x.y;
30
+ x.a = props.a;
31
+ $[0] = props.a;
32
+ $[1] = x;
33
+ } else {
34
+ x = $[1];
35
+ }
36
+ return x;
37
+}
38
+
39
+```
40
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-overlap-descendant.js
new
+9
@@ -0,0 +1,9 @@
1
+// Test that we correctly track a subpath if the subpath itself is accessed as
2
+// a dependency
3
+function TestOverlappingDescendantTracked(props) {
4
+ let x = {};
5
+ x.b = props.a.b.c;
6
+ x.c = props.a.b.c.x.y;
7
+ x.a = props.a;
8
+ return x;
9
+}
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-overlap-direct.expect.md
new
+40
@@ -0,0 +1,40 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// Test that we correctly track a subpath if the subpath itself is accessed as
6
+// a dependency
7
+function TestOverlappingTracked(props) {
8
+ let x = {};
9
+ x.b = props.a.b;
10
+ x.c = props.a.c;
11
+ x.a = props.a;
12
+ return x;
13
+}
14
+
15
+```
16
+
17
+## Code
18
+
19
+```javascript
20
+// Test that we correctly track a subpath if the subpath itself is accessed as
21
+// a dependency
22
+function TestOverlappingTracked(props) {
23
+ const $ = React.unstable_useMemoCache(2);
24
+ const c_0 = $[0] !== props.a;
25
+ let x;
26
+ if (c_0) {
27
+ x = {};
28
+ x.b = props.a.b;
29
+ x.c = props.a.c;
30
+ x.a = props.a;
31
+ $[0] = props.a;
32
+ $[1] = x;
33
+ } else {
34
+ x = $[1];
35
+ }
36
+ return x;
37
+}
38
+
39
+```
40
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-overlap-direct.js
new
+9
@@ -0,0 +1,9 @@
1
+// Test that we correctly track a subpath if the subpath itself is accessed as
2
+// a dependency
3
+function TestOverlappingTracked(props) {
4
+ let x = {};
5
+ x.b = props.a.b;
6
+ x.c = props.a.c;
7
+ x.a = props.a;
8
+ return x;
9
+}
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-subpath-order1.expect.md
new
+40
@@ -0,0 +1,40 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// Determine that we only need to track p.a here
6
+// Ordering of access should not matter
7
+function TestDepsSubpathOrder1(props) {
8
+ let x = {};
9
+ x.b = props.a.b;
10
+ x.a = props.a;
11
+ x.c = props.a.b.c;
12
+ return x;
13
+}
14
+
15
+```
16
+
17
+## Code
18
+
19
+```javascript
20
+// Determine that we only need to track p.a here
21
+// Ordering of access should not matter
22
+function TestDepsSubpathOrder1(props) {
23
+ const $ = React.unstable_useMemoCache(2);
24
+ const c_0 = $[0] !== props.a;
25
+ let x;
26
+ if (c_0) {
27
+ x = {};
28
+ x.b = props.a.b;
29
+ x.a = props.a;
30
+ x.c = props.a.b.c;
31
+ $[0] = props.a;
32
+ $[1] = x;
33
+ } else {
34
+ x = $[1];
35
+ }
36
+ return x;
37
+}
38
+
39
+```
40
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-subpath-order1.js
new
+9
@@ -0,0 +1,9 @@
1
+// Determine that we only need to track p.a here
2
+// Ordering of access should not matter
3
+function TestDepsSubpathOrder1(props) {
4
+ let x = {};
5
+ x.b = props.a.b;
6
+ x.a = props.a;
7
+ x.c = props.a.b.c;
8
+ return x;
9
+}
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-subpath-order2.expect.md
new
+40
@@ -0,0 +1,40 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// Determine that we only need to track p.a here
6
+// Ordering of access should not matter
7
+function TestDepsSubpathOrder2(props) {
8
+ let x = {};
9
+ x.a = props.a;
10
+ x.b = props.a.b;
11
+ x.c = props.a.b.c;
12
+ return x;
13
+}
14
+
15
+```
16
+
17
+## Code
18
+
19
+```javascript
20
+// Determine that we only need to track p.a here
21
+// Ordering of access should not matter
22
+function TestDepsSubpathOrder2(props) {
23
+ const $ = React.unstable_useMemoCache(2);
24
+ const c_0 = $[0] !== props.a;
25
+ let x;
26
+ if (c_0) {
27
+ x = {};
28
+ x.a = props.a;
29
+ x.b = props.a.b;
30
+ x.c = props.a.b.c;
31
+ $[0] = props.a;
32
+ $[1] = x;
33
+ } else {
34
+ x = $[1];
35
+ }
36
+ return x;
37
+}
38
+
39
+```
40
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-subpath-order2.js
new
+9
@@ -0,0 +1,9 @@
1
+// Determine that we only need to track p.a here
2
+// Ordering of access should not matter
3
+function TestDepsSubpathOrder2(props) {
4
+ let x = {};
5
+ x.a = props.a;
6
+ x.b = props.a.b;
7
+ x.c = props.a.b.c;
8
+ return x;
9
+}
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-subpath-order3.expect.md
new
+40
@@ -0,0 +1,40 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// Determine that we only need to track p.a here
6
+// Ordering of access should not matter
7
+function TestDepsSubpathOrder3(props) {
8
+ let x = {};
9
+ x.c = props.a.b.c;
10
+ x.a = props.a;
11
+ x.b = props.a.b;
12
+ return x;
13
+}
14
+
15
+```
16
+
17
+## Code
18
+
19
+```javascript
20
+// Determine that we only need to track p.a here
21
+// Ordering of access should not matter
22
+function TestDepsSubpathOrder3(props) {
23
+ const $ = React.unstable_useMemoCache(2);
24
+ const c_0 = $[0] !== props.a;
25
+ let x;
26
+ if (c_0) {
27
+ x = {};
28
+ x.c = props.a.b.c;
29
+ x.a = props.a;
30
+ x.b = props.a.b;
31
+ $[0] = props.a;
32
+ $[1] = x;
33
+ } else {
34
+ x = $[1];
35
+ }
36
+ return x;
37
+}
38
+
39
+```
40
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/reduce-reactive-uncond-deps-subpath-order3.js
new
+9
@@ -0,0 +1,9 @@
1
+// Determine that we only need to track p.a here
2
+// Ordering of access should not matter
3
+function TestDepsSubpathOrder3(props) {
4
+ let x = {};
5
+ x.c = props.a.b.c;
6
+ x.a = props.a;
7
+ x.b = props.a.b;
8
+ return x;
9
+}