@samitouri / QOS-React-2 / commits / 5b0f566941

Example of "unnecessary" memoization of lambdas

I noticed this while demoing Forget to React Org alum Christoph Nakazawa — in array.map calls (and other APIs that take a lambda as input) we sometimes end up memoizing the lambda. It's technically correct since the function _could_ return the lambda, and then we'd need it to be memoized. It's tricky because array.map is often called on nested objects, where even if we had type inference on the outer value we wouldn't know for sure that the inner property is an Array and not some other data type with a custom .map. For example in `data.feedback.comments.edges.map(edge => ...)`, even if we knew that `data` was an Object, we wouldn't know that data.feedback.comments.edges is an Array without cross-file type knowledge. But it's definitely wasteful to memoize these lambdas, so we should brainstorm options. One option that stands out right away: if the lambda has zero dependencies, then we could lift it out to module scope and refer to it by name.

Joe Savona committed May 10, 2023 at 23:09 UTC 5b0f56694125506623bdd9799c23378bb15e637f
2 files changed +73
compiler/forget/src/__tests__/fixtures/compiler/todo.unnecessary-lambda-memoization.expect.md new
+59
@@ -0,0 +1,59 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + const data = useFreeze(); // assume this returns {items: Array<{...}>}
7 + // In this call `data` and `data.items` have a read effect *and* the lambda itself
8 + // is readonly (it doesn't capture ony mutable references). Further, we ca
9 + // theoretically determine that the lambda doesn't need to be memoized, since
10 + // data.items is an Array and Array.prototype.map does not capture its input (callback)
11 + // in the return value.
12 + // An observation is that even without knowing the exact type of `data`, if we know
13 + // that it is a plain, readonly javascript object, then we can infer that any `.map()`
14 + // calls *must* be Array.prototype.map (or else they are a runtime error), since no
15 + // other builtin has a .map() function.
16 + const items = data.items.map((item) => <Item item={item} />);
17 + return <div>{items}</div>;
18 +}
19 +
20 +```
21 +
22 +## Code
23 +
24 +```javascript
25 +import { unstable_useMemoCache as useMemoCache } from "react";
26 +function Component(props) {
27 + const $ = useMemoCache(5);
28 + const data = useFreeze();
29 + const c_0 = $[0] !== data.items;
30 + let t1;
31 + if (c_0) {
32 + let t0;
33 + if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
34 + t0 = (item) => <Item item={item} />;
35 + $[2] = t0;
36 + } else {
37 + t0 = $[2];
38 + }
39 + t1 = data.items.map(t0);
40 + $[0] = data.items;
41 + $[1] = t1;
42 + } else {
43 + t1 = $[1];
44 + }
45 + const items = t1;
46 + const c_3 = $[3] !== items;
47 + let t2;
48 + if (c_3) {
49 + t2 = <div>{items}</div>;
50 + $[3] = items;
51 + $[4] = t2;
52 + } else {
53 + t2 = $[4];
54 + }
55 + return t2;
56 +}
57 +
58 +```
59 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/todo.unnecessary-lambda-memoization.js new
+14
@@ -0,0 +1,14 @@
1 +function Component(props) {
2 + const data = useFreeze(); // assume this returns {items: Array<{...}>}
3 + // In this call `data` and `data.items` have a read effect *and* the lambda itself
4 + // is readonly (it doesn't capture ony mutable references). Further, we ca
5 + // theoretically determine that the lambda doesn't need to be memoized, since
6 + // data.items is an Array and Array.prototype.map does not capture its input (callback)
7 + // in the return value.
8 + // An observation is that even without knowing the exact type of `data`, if we know
9 + // that it is a plain, readonly javascript object, then we can infer that any `.map()`
10 + // calls *must* be Array.prototype.map (or else they are a runtime error), since no
11 + // other builtin has a .map() function.
12 + const items = data.items.map((item) => <Item item={item} />);
13 + return <div>{items}</div>;
14 +}