Initialize useMemoCache with sentinel values

[josephsavona]

Summary

EDIT: We ended up taking a different approach based on feedback, the description reflects the revised version.

The compiler needs to be able to insert checks to see if a given memo slot has been initialized yet. This PR defines a sentinel value as a Symbol, and fills all memo cache arrays with that symbol value. Eventually we can export the symbol, but for now i didn't want to add any public exports to React. We can use Symbol.for with the same key to get the sentinel value.

How did you test this change?

• useMemoCache-test

[josephsavona]

@kassens @poteto @gsathya Thinking about how we use the cache:

const c_x = !cache.has(0) || cache.get(0) !== x;
cache.set(0, x);

We'd probably be better off with a single function update() or similar:

const c_x = cache.update(0, x);

// Atomically update the value and return true if it was first initialized or changed,
// false if no update occurred.
function update(index, value) {
  // todo bounds check
  const previous = this.data[index];
  this.data[index] = value;
  return previous === sentinel || previous !== value;
}

[sizebot]

Comparing: 3b81432...25f6c1f

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.min.js	=	135.64 kB	135.64 kB	=	43.46 kB	43.46 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.04%	149.89 kB	149.95 kB	+0.09%	47.87 kB	47.92 kB
facebook-www/ReactDOM-prod.classic.js	+0.04%	492.27 kB	492.46 kB	+0.08%	87.58 kB	87.65 kB
facebook-www/ReactDOM-prod.modern.js	+0.04%	477.52 kB	477.72 kB	+0.08%	85.38 kB	85.45 kB
facebook-www/ReactDOMForked-prod.classic.js	+0.04%	492.27 kB	492.46 kB	+0.08%	87.58 kB	87.66 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
facebook-relay/flight/ReactFlightNativeRelayServer-prod.js	+0.65%	25.08 kB	25.24 kB	+0.91%	6.07 kB	6.13 kB
oss-stable-semver/react-server/cjs/react-server-flight.production.min.js	+0.63%	11.84 kB	11.92 kB	+0.80%	4.39 kB	4.42 kB
oss-stable/react-server/cjs/react-server-flight.production.min.js	+0.63%	11.84 kB	11.92 kB	+0.80%	4.39 kB	4.42 kB
oss-experimental/react-server/cjs/react-server-flight.production.min.js	+0.61%	12.40 kB	12.48 kB	+0.73%	4.53 kB	4.57 kB
facebook-www/ReactFlightDOMRelayServer-prod.classic.js	+0.50%	32.12 kB	32.28 kB	+0.66%	8.17 kB	8.22 kB
facebook-www/ReactFlightDOMRelayServer-prod.modern.js	+0.50%	32.20 kB	32.36 kB	+0.66%	8.20 kB	8.25 kB
facebook-relay/flight/ReactFlightNativeRelayServer-dev.js	+0.39%	47.01 kB	47.19 kB	+0.41%	11.96 kB	12.00 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.browser.production.min.server.js	+0.37%	20.40 kB	20.48 kB	+0.40%	7.22 kB	7.25 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.browser.production.min.server.js	+0.37%	20.40 kB	20.48 kB	+0.40%	7.22 kB	7.25 kB
oss-stable-semver/react-server/cjs/react-server-flight.development.js	+0.37%	49.27 kB	49.46 kB	+0.45%	12.34 kB	12.40 kB
oss-stable/react-server/cjs/react-server-flight.development.js	+0.37%	49.27 kB	49.46 kB	+0.45%	12.34 kB	12.40 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.node.production.min.server.js	+0.37%	20.82 kB	20.90 kB	+0.47%	7.31 kB	7.34 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.node.production.min.server.js	+0.37%	20.82 kB	20.90 kB	+0.47%	7.31 kB	7.34 kB
oss-stable-semver/react-server-dom-webpack/umd/react-server-dom-webpack-writer.browser.production.min.server.js	+0.36%	20.61 kB	20.69 kB	+0.44%	7.30 kB	7.33 kB
oss-stable/react-server-dom-webpack/umd/react-server-dom-webpack-writer.browser.production.min.server.js	+0.36%	20.61 kB	20.69 kB	+0.44%	7.30 kB	7.33 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.browser.production.min.server.js	+0.36%	21.00 kB	21.08 kB	+0.47%	7.39 kB	7.43 kB
oss-experimental/react-server-dom-webpack/umd/react-server-dom-webpack-writer.browser.production.min.server.js	+0.35%	21.20 kB	21.27 kB	+0.48%	7.47 kB	7.50 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.node.production.min.server.js	+0.35%	21.42 kB	21.50 kB	+0.43%	7.48 kB	7.51 kB
oss-experimental/react-server/cjs/react-server-flight.development.js	+0.34%	53.18 kB	53.36 kB	+0.44%	13.31 kB	13.37 kB
oss-experimental/react-server/cjs/react-server.production.min.js	+0.31%	23.85 kB	23.93 kB	+0.45%	8.05 kB	8.09 kB
facebook-www/ReactFlightDOMRelayServer-dev.classic.js	+0.27%	66.88 kB	67.06 kB	+0.23%	16.82 kB	16.86 kB
facebook-www/ReactFlightDOMRelayServer-dev.modern.js	+0.27%	66.94 kB	67.12 kB	+0.23%	16.84 kB	16.87 kB
oss-stable-semver/react-server-dom-webpack/umd/react-server-dom-webpack-writer.browser.development.server.js	+0.23%	84.79 kB	84.99 kB	+0.39%	20.64 kB	20.72 kB
oss-stable/react-server-dom-webpack/umd/react-server-dom-webpack-writer.browser.development.server.js	+0.23%	84.79 kB	84.99 kB	+0.39%	20.64 kB	20.72 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.browser.development.server.js	+0.23%	80.79 kB	80.97 kB	+0.39%	20.39 kB	20.47 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.browser.development.server.js	+0.23%	80.79 kB	80.97 kB	+0.39%	20.39 kB	20.47 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.node.development.server.js	+0.22%	82.22 kB	82.40 kB	+0.38%	20.52 kB	20.60 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.node.development.server.js	+0.22%	82.22 kB	82.40 kB	+0.38%	20.52 kB	20.60 kB
oss-experimental/react-server-dom-webpack/umd/react-server-dom-webpack-writer.browser.development.server.js	+0.22%	88.95 kB	89.14 kB	+0.36%	21.63 kB	21.71 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.browser.development.server.js	+0.21%	84.75 kB	84.93 kB	+0.34%	21.37 kB	21.45 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-writer.node.development.server.js	+0.21%	86.18 kB	86.36 kB	+0.33%	21.50 kB	21.57 kB

Generated by 🚫 dangerJS against 25f6c1f

[gsathya]

not sure how much we care about perf/memory here, but one trick is to just use the cache object as an array and index into it directly rather than creating a separate data array

[sebmarkbage]

I'm pretty concerned about the overhead here. The only thing that made me comfortable about the idea of memoizing everything in first place was the very low overhead that we could get by specializing the code and guarantee inlined operations.

If this is a temporary state, what's the plausible next step to getting it back?

[josephsavona]

@sebmarkbage I've completed revised this to just fill the cache with a sentinel value, can you take another look?

[kassens]

Thanks for the update!

[poteto]

I was curious about the performance of fill and ran a microbenchmark: https://jsbench.me/7ol98ws520/1

Looks like the fastest way is:

data = memoCache.data[memoCache.index] = new Array(size);
for (let i = 0; i < size; ++i) data[i] = MEMO_CACHE_UNSET_SENTINEL;

[josephsavona]

good call, i updated to use this implementation

[Huxpro]

Yea I agree that for the production scenario we should do things as low-level as possible.

I forget about how we do guardMemoCacheRead for the patched implementation. I guess we switched the implementation of useMemoCache hooks?

Maybe we can also fork the implementation for React dev build, or actually may be only the debug build.

[Huxpro]

Yea, I originally chose the $._ approach merely to ease the implementation of codegen and patching. It's likely to be faster if we can directly export this symbol to not only save one hidden class transition/modification of array but also one property lookup in the runtime.

The only caveat I had thought about is that we need to deal with potential name clashes everywhere which is hairy:

import { MEMO_CACHE_UNSET_SENTINEL as _ } from "react"

function Foo() {
  // ok
}

function Bar() {
  let _ = otherThing;
  // well
}

[Huxpro]

I originally thought of putting this to https://github.com/facebook/react/blob/main/packages/shared/ReactSymbols.js