Nested functions bindings (#2117)

Summary:
Release notes: fixes serialization of bindings when shared between optimized functions

This PR fixes a long standing bug with nested optimized functions where bindings would incorrectly serialize to the main body scope when the binding was shared between nested optimized functions and their parent optimized function scopes. This allows us to not skip 2 React tests and I also added a serializer regression test that demonstrates the same problem so we don't regress.
Closes https://github.com/facebook/prepack/pull/2117

Differential Revision: D8408712

Pulled By: trueadm

fbshipit-source-id: 0e0a306f9195fcb5b57fa7f8cef1c65dbef4998f
This commit is contained in:
Dominic Gannaway 2018-06-13 15:59:30 -07:00 committed by Facebook Github Bot
parent df757a6a28
commit 0d7807d871
6 changed files with 378 additions and 20 deletions

View File

@ -318,6 +318,54 @@ ReactStatistics {
}
`;
exports[`Test React with JSX input, JSX output Class component folding Simple classes with Array.from 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "A",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 1,
"optimizedTrees": 1,
}
`;
exports[`Test React with JSX input, JSX output Class component folding Simple classes with Array.from 2 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "A",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 1,
"optimizedTrees": 1,
}
`;
exports[`Test React with JSX input, JSX output Factory class component folding Simple factory classes 1`] = `
ReactStatistics {
"componentsEvaluated": 1,
@ -639,6 +687,30 @@ ReactStatistics {
}
`;
exports[`Test React with JSX input, JSX output First render only Replace this in callbacks 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "Child1",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 0,
"optimizedTrees": 1,
}
`;
exports[`Test React with JSX input, JSX output First render only Replace this in callbacks 2 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
@ -5179,6 +5251,54 @@ ReactStatistics {
}
`;
exports[`Test React with JSX input, create-element output Class component folding Simple classes with Array.from 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "A",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 1,
"optimizedTrees": 1,
}
`;
exports[`Test React with JSX input, create-element output Class component folding Simple classes with Array.from 2 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "A",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 1,
"optimizedTrees": 1,
}
`;
exports[`Test React with JSX input, create-element output Factory class component folding Simple factory classes 1`] = `
ReactStatistics {
"componentsEvaluated": 1,
@ -5500,6 +5620,30 @@ ReactStatistics {
}
`;
exports[`Test React with JSX input, create-element output First render only Replace this in callbacks 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "Child1",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 0,
"optimizedTrees": 1,
}
`;
exports[`Test React with JSX input, create-element output First render only Replace this in callbacks 2 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
@ -10040,6 +10184,54 @@ ReactStatistics {
}
`;
exports[`Test React with create-element input, JSX output Class component folding Simple classes with Array.from 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "A",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 1,
"optimizedTrees": 1,
}
`;
exports[`Test React with create-element input, JSX output Class component folding Simple classes with Array.from 2 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "A",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 1,
"optimizedTrees": 1,
}
`;
exports[`Test React with create-element input, JSX output Factory class component folding Simple factory classes 1`] = `
ReactStatistics {
"componentsEvaluated": 1,
@ -10361,6 +10553,30 @@ ReactStatistics {
}
`;
exports[`Test React with create-element input, JSX output First render only Replace this in callbacks 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "Child1",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 0,
"optimizedTrees": 1,
}
`;
exports[`Test React with create-element input, JSX output First render only Replace this in callbacks 2 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
@ -14916,6 +15132,54 @@ ReactStatistics {
}
`;
exports[`Test React with create-element input, create-element output Class component folding Simple classes with Array.from 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "A",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 1,
"optimizedTrees": 1,
}
`;
exports[`Test React with create-element input, create-element output Class component folding Simple classes with Array.from 2 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "A",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 1,
"optimizedTrees": 1,
}
`;
exports[`Test React with create-element input, create-element output Factory class component folding Simple factory classes 1`] = `
ReactStatistics {
"componentsEvaluated": 1,
@ -15237,6 +15501,30 @@ ReactStatistics {
}
`;
exports[`Test React with create-element input, create-element output First render only Replace this in callbacks 1`] = `
ReactStatistics {
"componentsEvaluated": 2,
"evaluatedRootNodes": Array [
Object {
"children": Array [
Object {
"children": Array [],
"message": "",
"name": "Child1",
"status": "INLINED",
},
],
"message": "",
"name": "App",
"status": "ROOT",
},
],
"inlinedComponents": 1,
"optimizedNestedClosures": 0,
"optimizedTrees": 1,
}
`;
exports[`Test React with create-element input, create-element output First render only Replace this in callbacks 2 1`] = `
ReactStatistics {
"componentsEvaluated": 2,

View File

@ -797,16 +797,11 @@ function runTestSuite(outputJsx, shouldTranspileSource) {
await runTest(directory, "simple-classes-3.js");
});
// awaiting PR on nested additional support #1626,
// issues is that both the parent and child additional
// function share the same variable, so the serializer
// incorrectly emits it in the MainGenerator scope
it.skip("Simple classes with Array.from", async () => {
it("Simple classes with Array.from", async () => {
await runTest(directory, "array-from.js");
});
// same issue as last test
it.skip("Simple classes with Array.from 2", async () => {
it("Simple classes with Array.from 2", async () => {
await runTest(directory, "array-from2.js");
});

View File

@ -78,7 +78,7 @@ import { canHoistFunction } from "../react/hoisting.js";
import { To } from "../singletons.js";
import { ResidualReactElementSerializer } from "./ResidualReactElementSerializer.js";
import type { Binding } from "../environment.js";
import { DeclarativeEnvironmentRecord } from "../environment.js";
import { DeclarativeEnvironmentRecord, FunctionEnvironmentRecord } from "../environment.js";
import type { Referentializer } from "./Referentializer.js";
import { GeneratorDAG } from "./GeneratorDAG.js";
import { type Replacement, getReplacement } from "./ResidualFunctionInstantiator";
@ -695,10 +695,12 @@ export class ResidualHeapSerializer {
let result = new Set(initialGenerators);
let activeFunctions = functionValues.slice();
let visitedFunctions = new Set();
while (activeFunctions.length > 0) {
let f = activeFunctions.pop();
if (visitedFunctions.has(f)) continue;
visitedFunctions.add(f);
if (f === referencingOnlyOptimizedFunction) {
let g = this.additionalFunctionGenerators.get(f);
invariant(g !== undefined);
@ -718,11 +720,29 @@ export class ResidualHeapSerializer {
return Array.from(result);
}
// Determine if a value is effectively referenced by a single additional function.
isReferencedOnlyByOptimizedFunction(val: Value): void | FunctionValue {
isDefinedInsideFunction(childFunction: FunctionValue, maybeParentFunctions: Set<FunctionValue>): boolean {
for (let maybeParentFunction of maybeParentFunctions) {
if (childFunction === maybeParentFunction) {
continue;
}
let env = childFunction.$Environment;
while (env.parent !== null) {
let envRecord = env.environmentRecord;
if (envRecord instanceof FunctionEnvironmentRecord && envRecord.$FunctionObject === maybeParentFunction) {
return true;
}
env = env.parent;
}
}
return false;
}
// Try and get the root optimized function when passed in an optimized function
// that may or may not be nested in the tree of said root, or is the root optimized function
tryGetOptimizedFunctionRoot(val: Value): void | FunctionValue {
let scopes = this.residualValues.get(val);
let functionValues = new Set();
invariant(scopes !== undefined);
let additionalFunction;
for (let scope of scopes) {
let s = scope;
while (s instanceof Generator) {
@ -730,11 +750,21 @@ export class ResidualHeapSerializer {
}
if (s === "GLOBAL") return undefined;
invariant(s instanceof FunctionValue);
if (this.additionalFunctionGenerators.has(s)) {
if (additionalFunction !== undefined && additionalFunction !== s) return undefined;
additionalFunction = s;
functionValues.add(s);
}
let additionalFunction;
for (let functionValue of functionValues) {
if (this.additionalFunctionGenerators.has(functionValue)) {
if (this.isDefinedInsideFunction(functionValue, functionValues)) {
continue;
}
if (additionalFunction !== undefined && additionalFunction !== functionValue) {
return undefined;
}
additionalFunction = functionValue;
} else {
let f = this.isReferencedOnlyByOptimizedFunction(s);
let f = this.tryGetOptimizedFunctionRoot(functionValue);
if (f === undefined) return undefined;
if (additionalFunction !== undefined && additionalFunction !== f) return undefined;
additionalFunction = f;
@ -780,7 +810,7 @@ export class ResidualHeapSerializer {
}
}
let referencingOnlyOptimizedFunction = this.isReferencedOnlyByOptimizedFunction(val);
let referencingOnlyOptimizedFunction = this.tryGetOptimizedFunctionRoot(val);
if (generators.length === 0) {
// This value is only referenced from residual functions.
if (
@ -839,6 +869,11 @@ export class ResidualHeapSerializer {
(x, y) => commonAncestorOf(x, y, getGeneratorParent),
generators[0]
);
// In the case where we have no common ancestor but we have an optimized function reference,
// we can attempt to use the generator of the single optimized function
if (commonAncestor === undefined && referencingOnlyOptimizedFunction !== undefined) {
commonAncestor = this.additionalFunctionGenerators.get(referencingOnlyOptimizedFunction);
}
invariant(commonAncestor !== undefined, "there must always be a common generator ancestor");
if (trace) console.log(` common ancestor: ${commonAncestor.getName()}`);
@ -1393,7 +1428,7 @@ export class ResidualHeapSerializer {
invariant(instance !== undefined);
let residualBindings = instance.residualFunctionBindings;
let inOptimizedFunction = this.isReferencedOnlyByOptimizedFunction(val);
let inOptimizedFunction = this.tryGetOptimizedFunctionRoot(val);
if (inOptimizedFunction !== undefined) instance.containingAdditionalFunction = inOptimizedFunction;
let bindingsEmittedSemaphore = new CountingSemaphore(() => {
invariant(instance);
@ -2141,7 +2176,7 @@ export class ResidualHeapSerializer {
functionValue: FunctionValue,
additionalEffects: AdditionalFunctionEffects
) {
let inAdditionalFunction = this.isReferencedOnlyByOptimizedFunction(functionValue);
let inAdditionalFunction = this.tryGetOptimizedFunctionRoot(functionValue);
return this._withGeneratorScope(
"AdditionalFunction",
generator,

View File

@ -97,7 +97,7 @@ export class ResidualReactElementSerializer {
t.callExpression(funcId, originalCreateElementIdentifier ? [originalCreateElementIdentifier] : [])
)
);
let optimizedFunction = this.residualHeapSerializer.isReferencedOnlyByOptimizedFunction(reactElement);
let optimizedFunction = this.residualHeapSerializer.tryGetOptimizedFunctionRoot(reactElement);
this.residualHeapSerializer.getPrelude(optimizedFunction).push(statement);
}
// we then push the reactElement and its id into our list of elements to process after
@ -129,7 +129,7 @@ export class ResidualReactElementSerializer {
let propsValue = getProperty(this.realm, value, "props");
let shouldHoist =
this.residualHeapSerializer.isReferencedOnlyByOptimizedFunction(value) !== undefined &&
this.residualHeapSerializer.tryGetOptimizedFunctionRoot(value) !== undefined &&
canHoistReactElement(this.realm, value);
let id = this.residualHeapSerializer.getSerializeObjectIdentifier(value);

View File

@ -0,0 +1,19 @@
(function () {
function fn() {
var self = global.__abstract ? global.__abstract("object", "this") : this;
var fn2 = function(item) {
return [item, self];
}
if (global.__optimize) __optimize(fn2);
return {fn2, self};
}
if (global.__optimize) __optimize(fn);
global.inspect = function() {
var result = fn.call({a: 1})
return JSON.stringify(result.fn2())
}
})();

View File

@ -0,0 +1,21 @@
(function () {
var fn2 = function(item, self) {
return [item, self];
}
function fn() {
var self = global.__abstract ? global.__abstract("object", "this") : this;
var fn3 = fn2.bind(self);
if (global.__optimize) __optimize(fn3);
return {fn3, self};
}
if (global.__optimize) __optimize(fn);
global.inspect = function() {
var result = fn.call({a: 1})
return JSON.stringify(result.fn2())
}
})();