Optimize ReactEquivalenceSet (#2243)

Summary:
I generated the following program (now added to test cases) and while I would expect its evaluation in Prepack to terminate reasonably quickly, I did not see the program terminate after five minutes of execution:

https://gist.github.com/calebmer/2bf1d84a5b7849fa732bce69811df10b

After this change that same program’s Prepack evaluation terminates in about two seconds. This change also saves about 2.8s on the evaluation of our internal bundle which is about 3% of the total time.

What was happening? Why did my program fail to terminate execution in five minutes? Why was the internal bundle relatively ok compared to my extreme case?

In my program, I would [expect the component `Mu`](https://gist.github.com/calebmer/2bf1d84a5b7849fa732bce69811df10b#file-program-js-L289-L291) to be inlined about 60 times. Which means there should only be about 60 calls to `ReactElementSet#add` for each of `Mu`’s `div`s. However, after some basic instrumentation I observed way over ten thousand visits to these React elements.

This pair of method calls happens a couple times in `ReactEquivalenceSet` and friends.

5f7256f17e/src/react/ReactEquivalenceSet.js (L233-L234)

Both `getEquivalentPropertyValue`…

5f7256f17e/src/react/ReactEquivalenceSet.js (L255)

…and `getValue`…

5f7256f17e/src/react/ReactEquivalenceSet.js (L104)

…end up calling `ReactElementSet#add` with the same React element. Then `ReactElementSet#add` ends up recursing into children. So the root element is added 2 times, so the root’s children are added 4 times each, so each child of those elements are added 8 times each, and so on. So if a leaf element is `n` deep in the tree it will be added `2^n` times. The most nested `Mu` child was 9 elements deep. `Mu` appeared 60 times at many levels of nesting.

I’m not entirely sure why my generated case was so much worse then our internal bundle. My two best guesses are: lots of repeated components or deeper nesting?

My fix was to move the `getValue` call into `getEquivalentPropertyValue`. This way if `getEquivalentPropertyValue` already finds an equivalent value we can short-circuit and not run `getValue` which will perform the same operation.
Pull Request resolved: https://github.com/facebook/prepack/pull/2243

Reviewed By: trueadm

Differential Revision: D8811463

Pulled By: calebmer

fbshipit-source-id: 4f30a75e96c6592456f5b21ee9c2ee3e40b56d81
This commit is contained in:
Caleb Meredith 2018-07-13 13:32:13 -07:00 committed by Facebook Github Bot
parent 1a4ab4e89d
commit 37d692480b
7 changed files with 5415 additions and 27 deletions

1
.gitignore vendored
View File

@ -3,6 +3,7 @@
/node_modules
/lib
npm-debug.log
yarn-error.log
.DS_Store
build/
.idea/

View File

@ -29,23 +29,19 @@ export class ReactElementSet {
// type
currentMap = reactEquivalenceSet.getKey("type", currentMap, visitedValues);
let type = reactEquivalenceSet.getEquivalentPropertyValue(reactElement, "type");
let result = reactEquivalenceSet.getValue(type, currentMap, visitedValues);
let result = reactEquivalenceSet.getEquivalentPropertyValue(reactElement, "type", currentMap, visitedValues);
currentMap = result.map;
// key
currentMap = reactEquivalenceSet.getKey("key", currentMap, visitedValues);
let key = reactEquivalenceSet.getEquivalentPropertyValue(reactElement, "key");
result = reactEquivalenceSet.getValue(key, currentMap, visitedValues);
result = reactEquivalenceSet.getEquivalentPropertyValue(reactElement, "key", currentMap, visitedValues);
currentMap = result.map;
// ref
currentMap = reactEquivalenceSet.getKey("ref", currentMap, visitedValues);
let ref = reactEquivalenceSet.getEquivalentPropertyValue(reactElement, "ref");
result = reactEquivalenceSet.getValue(ref, currentMap, visitedValues);
result = reactEquivalenceSet.getEquivalentPropertyValue(reactElement, "ref", currentMap, visitedValues);
currentMap = result.map;
// props
currentMap = reactEquivalenceSet.getKey("props", currentMap, visitedValues);
let props = reactEquivalenceSet.getEquivalentPropertyValue(reactElement, "props");
result = reactEquivalenceSet.getValue(props, currentMap, visitedValues);
result = reactEquivalenceSet.getEquivalentPropertyValue(reactElement, "props", currentMap, visitedValues);
if (result.value === null) {
result.value = reactElement;

View File

@ -79,7 +79,7 @@ export class ReactEquivalenceSet {
return ((map.get(key): any): ReactSetValueMap);
}
getValue(val: ReactSetValueMapKey, map: ReactSetValueMap, visitedValues: Set<Value>): ReactSetNode {
_getValue(val: ReactSetValueMapKey, map: ReactSetValueMap, visitedValues: Set<Value>): ReactSetNode {
if (val instanceof StringValue || val instanceof NumberValue) {
val = val.value;
} else if (val instanceof AbstractValue) {
@ -108,14 +108,13 @@ export class ReactEquivalenceSet {
for (let [propName] of object.properties) {
currentMap = this.getKey(propName, currentMap, visitedValues);
let prop = this.getEquivalentPropertyValue(object, propName);
result = this.getValue(prop, currentMap, visitedValues);
result = this.getEquivalentPropertyValue(object, propName, currentMap, visitedValues);
currentMap = result.map;
}
for (let [symbol] of object.symbols) {
currentMap = this.getKey(symbol, currentMap, visitedValues);
let prop = getProperty(this.realm, object, symbol);
result = this.getValue(prop, currentMap, visitedValues);
result = this._getValue(prop, currentMap, visitedValues);
currentMap = result.map;
}
let temporalAlias = object.temporalAlias;
@ -188,7 +187,7 @@ export class ReactEquivalenceSet {
}
currentMap = this.getKey(i, (currentMap: any), visitedValues);
invariant(arg instanceof Value && (equivalenceArg instanceof Value || equivalenceArg === undefined));
result = this.getValue(equivalenceArg || arg, currentMap, visitedValues);
result = this._getValue(equivalenceArg || arg, currentMap, visitedValues);
currentMap = result.map;
}
invariant(result !== undefined);
@ -230,8 +229,7 @@ export class ReactEquivalenceSet {
for (let i = 0; i < length; i++) {
currentMap = this.getKey(i, currentMap, visitedValues);
let element = this.getEquivalentPropertyValue(array, "" + i);
result = this.getValue(element, currentMap, visitedValues);
result = this.getEquivalentPropertyValue(array, "" + i, currentMap, visitedValues);
currentMap = result.map;
}
if (result === undefined) {
@ -246,30 +244,34 @@ export class ReactEquivalenceSet {
return result.value;
}
getEquivalentPropertyValue(object: ObjectValue, propName: string): Value {
getEquivalentPropertyValue(
object: ObjectValue,
propName: string,
map: ReactSetValueMap,
visitedValues: Set<Value>
): ReactSetNode {
let prop = getProperty(this.realm, object, propName);
let isFinal = object.mightBeFinalObject();
let equivalentProp;
if (prop instanceof ObjectValue && isReactElement(prop)) {
equivalentProp = this.residualReactElementVisitor.reactElementEquivalenceSet.add(prop);
if (prop !== equivalentProp && isFinal) {
hardModifyReactObjectPropertyBinding(this.realm, object, propName, equivalentProp);
}
} else if (prop instanceof ObjectValue && isReactPropsObject(prop)) {
equivalentProp = this.residualReactElementVisitor.reactPropsEquivalenceSet.add(prop);
if (prop !== equivalentProp && isFinal) {
hardModifyReactObjectPropertyBinding(this.realm, object, propName, equivalentProp);
}
} else if (prop instanceof AbstractValue) {
equivalentProp = this.residualReactElementVisitor.residualHeapVisitor.equivalenceSet.add(prop);
}
if (equivalentProp !== undefined) {
if (prop !== equivalentProp && isFinal) {
hardModifyReactObjectPropertyBinding(this.realm, object, propName, equivalentProp);
}
if (!map.has(equivalentProp)) {
map.set(equivalentProp, this._createNode());
}
return ((map.get(equivalentProp): any): ReactSetNode);
} else {
return this._getValue(prop, map, visitedValues);
}
return equivalentProp || prop;
}
}

View File

@ -30,8 +30,7 @@ export class ReactPropsSet {
for (let [propName] of props.properties) {
currentMap = reactEquivalenceSet.getKey(propName, currentMap, visitedValues);
let prop = reactEquivalenceSet.getEquivalentPropertyValue(props, propName);
result = reactEquivalenceSet.getValue(prop, currentMap, visitedValues);
result = reactEquivalenceSet.getEquivalentPropertyValue(props, propName, currentMap, visitedValues);
currentMap = result.map;
}
let temporalAlias = props.temporalAlias;

View File

@ -319,3 +319,7 @@ it("Dynamic ReactElement type #4", () => {
it("Hoist Fragment", () => {
runTest(__dirname + "/FunctionalComponents/hoist-fragment.js");
});
it("Pathological case", () => {
runTest(__dirname + "/FunctionalComponents/pathological-case.js");
});

View File

@ -0,0 +1,438 @@
const React = require("react");
function Gamma(props) {
return React.createElement(
React.Fragment,
null,
React.createElement(Theta, {
n: props.c,
w: props.c,
s: props.c,
q: props.x,
g: null,
a: props.x,
f: props.c,
}),
[
React.createElement(
"div",
{
key: "0",
"data-c": props.c,
},
React.createElement(Iota, {
d: props.c,
w: props.x,
u: props.c,
})
),
React.createElement(
"div",
{
key: "1",
},
React.createElement(Xi, {
v: props.x,
c: props.x,
e: props.c,
a: props.c,
z: props.x,
s: 3,
u: props.x,
n: props.x,
o: props.c,
j: true,
}),
React.createElement(Zeta, {
m: props.c,
u: props.c,
s: props.x,
}),
React.createElement(Epsilon, {
s: props.x,
c: props.c,
r: props.c,
q: props.c,
e: props.x,
g: props.c,
o: props.x,
h: props.c,
x: props.x,
})
),
],
React.createElement(
"div",
{
"data-x": props.x,
},
React.createElement(
"div",
{
"data-x": props.x,
},
React.createElement(Delta, {
l: null,
}),
React.createElement(Delta, {
l: props.x,
})
),
React.createElement(Xi, {
v: 1,
c: props.c,
e: props.c,
a: props.x,
z: props.x,
s: props.x,
u: props.c,
n: props.c,
o: props.c,
j: props.x,
}),
React.createElement("div", {
"data-x": props.x,
"data-c": props.c,
}),
React.createElement(Xi, {
v: props.c,
c: props.c,
e: props.c,
a: props.c,
z: null,
s: props.x,
u: props.x,
n: 3,
o: props.x,
j: -3,
})
),
React.createElement(
"div",
{
"data-c": props.c,
},
React.createElement(Iota, {
d: props.c,
w: props.c,
u: true,
}),
React.createElement(Delta, {
l: props.c,
}),
React.createElement(Iota, {
d: props.c,
w: -4,
u: props.c,
}),
React.createElement(Xi, {
v: props.c,
c: props.x,
e: props.x,
a: props.c,
z: props.c,
s: props.c,
u: props.c,
n: props.c,
o: null,
j: props.x,
})
),
React.createElement("div", {
"data-x": props.x,
})
);
}
function Delta(props) {
return React.createElement(
React.Fragment,
null,
React.createElement(
"div",
{},
React.createElement(Iota, {
d: props.l,
w: props.l,
u: props.l,
}),
React.createElement(Mu, {
v: props.l,
m: props.l,
}),
React.createElement(Epsilon, {
s: 6,
c: props.l,
r: props.l,
q: props.l,
e: props.l,
g: props.l,
o: props.l,
h: props.l,
x: props.l,
})
),
React.createElement(Pi, {
a: props.l,
h: props.l,
c: true,
z: props.l,
}),
null
);
}
function Iota(props) {
return React.createElement(
React.Fragment,
null,
React.createElement(
React.Fragment,
null,
React.createElement(
"div",
{},
React.createElement(
"div",
{
"data-w": props.w,
"data-d": props.d,
},
[
React.createElement(Pi, {
key: "0",
a: props.d,
h: props.u,
c: -13,
z: props.w,
}),
],
React.createElement("div", {
"data-d": props.d,
})
)
)
),
React.createElement(
"div",
{
"data-u": props.u,
"data-w": props.w,
"data-d": props.d,
},
React.createElement(Lambda, {
m: props.u,
v: props.w,
r: props.u,
}),
React.createElement(Xi, {
v: props.u,
c: props.d,
e: props.d,
a: -4,
z: props.u,
s: props.d,
u: props.u,
n: props.w,
o: props.u,
j: props.w,
}),
React.createElement(Xi, {
v: props.d,
c: 7,
e: props.u,
a: null,
z: props.u,
s: props.u,
u: props.u,
n: props.d,
o: -10,
j: props.w,
})
),
React.createElement(
React.Fragment,
null,
React.createElement(Zeta, {
m: props.d,
u: props.d,
s: props.u,
})
)
);
}
function Lambda(props) {
return React.createElement(Theta, {
n: props.m,
w: props.r,
s: props.v,
q: props.m,
g: props.v,
a: props.m,
f: props.r,
});
}
function Eta(props) {
return React.createElement(Theta, {
n: props.v,
w: props.s,
s: props.p,
q: props.q,
g: props.s,
a: null,
f: props.p,
});
}
function Epsilon(props) {
return React.createElement(
"div",
{
"data-x": props.x,
"data-e": props.e,
"data-c": props.c,
"data-s": props.s,
"data-q": props.q,
"data-h": props.h,
"data-g": props.g,
"data-o": props.o,
},
React.createElement("div", {
"data-g": props.g,
}),
React.createElement(React.Fragment, null, null),
React.createElement(Pi, {
a: props.c,
h: props.c,
c: props.r,
z: props.e,
})
);
}
function Pi(props) {
return React.createElement(
"div",
{
"data-a": props.a,
"data-h": props.h,
"data-c": props.c,
},
React.createElement("div", {
"data-h": props.h,
"data-z": props.z,
"data-c": props.c,
"data-a": props.a,
}),
React.createElement(Zeta, {
m: props.z,
u: props.z,
s: props.c,
})
);
}
function Theta(props) {
return React.createElement(React.Fragment, null, null);
}
function Xi(props) {
return React.createElement(
"div",
{
"data-j": props.j,
"data-v": props.v,
},
React.createElement(
"div",
{
"data-v": props.v,
"data-c": props.c,
"data-e": props.e,
"data-n": props.n,
"data-u": props.u,
"data-s": props.s,
"data-o": props.o,
},
React.createElement(
React.Fragment,
null,
React.createElement("div", {
"data-j": props.j,
"data-n": props.n,
})
)
)
);
}
function Zeta(props) {
return React.createElement(
"div",
{
"data-u": props.u,
},
React.createElement("div", {
"data-u": props.u,
"data-s": props.s,
}),
React.createElement(
"div",
{},
React.createElement(
"div",
{},
React.createElement(Mu, {
v: props.s,
m: props.s,
}),
React.createElement(Mu, {
v: props.u,
m: props.m,
}),
React.createElement(Mu, {
v: props.u,
m: props.s,
})
)
),
React.createElement(
"div",
{
"data-m": props.m,
"data-s": props.s,
},
React.createElement("div", {}),
React.createElement(
"div",
{},
React.createElement(
"div",
{
"data-s": props.s,
},
[]
)
)
)
);
}
function Mu(props) {
return React.createElement("div", {}, null, React.createElement("div", {}, null));
}
if (this.__optimizeReactComponentTree) {
__optimizeReactComponentTree(Gamma);
}
Gamma.getTrials = function(renderer, Gamma) {
let results = [];
renderer.update(<Gamma c={null} x={12} />);
results.push(["render pathological case", renderer.toJSON()]);
return results;
};
module.exports = Gamma;

File diff suppressed because it is too large Load Diff