From 2f5a6cb013d5723f8850512a60df89f5ee782b9e Mon Sep 17 00:00:00 2001 From: Dominic Gannaway <dg@domgan.com> Date: Thu, 9 Aug 2018 07:51:48 +0100 Subject: [PATCH 1/4] Adds a flag for Array.prototype method nested optimized functions --- scripts/debug-fb-www.js | 1 + scripts/test-runner.js | 1 + src/options.js | 1 + src/prepack-options.js | 3 +++ src/realm.js | 2 ++ src/values/ArrayValue.js | 2 +- test/react/setupReactTests.js | 1 + 7 files changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/debug-fb-www.js b/scripts/debug-fb-www.js index b4cc6b7a3f..a300f6d563 100644 --- a/scripts/debug-fb-www.js +++ b/scripts/debug-fb-www.js @@ -74,6 +74,7 @@ let prepackOptions = { reactOutput: "jsx", reactVerbose: true, reactOptimizeNestedFunctions: false, + arrayNestedOptimizedFunctionsEnabled: false, inlineExpressions: true, invariantLevel: 0, abstractValueImpliesMax: 1000, diff --git a/scripts/test-runner.js b/scripts/test-runner.js index c9f3a512a2..0af372b856 100644 --- a/scripts/test-runner.js +++ b/scripts/test-runner.js @@ -363,6 +363,7 @@ function runTest(name, code, options: PrepackOptions, args) { internalDebug: true, serialize: true, uniqueSuffix: "", + arrayNestedOptimizedFunctionsEnabled: true, }): any): PrepackOptions); // Since PrepackOptions is an exact type I have to cast if (code.includes("// throws introspection error")) { try { diff --git a/src/options.js b/src/options.js index fef529e1e0..d0217792e2 100644 --- a/src/options.js +++ b/src/options.js @@ -59,6 +59,7 @@ export type RealmOptions = { reactOptimizeNestedFunctions?: boolean, stripFlow?: boolean, abstractValueImpliesMax?: number, + arrayNestedOptimizedFunctionsEnabled?: boolean, }; export type SerializerOptions = { diff --git a/src/prepack-options.js b/src/prepack-options.js index 18ada43c0b..0660a38759 100644 --- a/src/prepack-options.js +++ b/src/prepack-options.js @@ -61,6 +61,7 @@ export type PrepackOptions = {| debuggerConfigArgs?: DebuggerConfigArguments, debugReproArgs?: DebugReproArguments, onParse?: BabelNodeFile => void, + arrayNestedOptimizedFunctionsEnabled?: boolean, |}; export function getRealmOptions({ @@ -87,6 +88,7 @@ export function getRealmOptions({ abstractValueImpliesMax, debuggerConfigArgs, debugReproArgs, + arrayNestedOptimizedFunctionsEnabled, }: PrepackOptions): RealmOptions { return { compatibility, @@ -112,6 +114,7 @@ export function getRealmOptions({ abstractValueImpliesMax, debuggerConfigArgs, debugReproArgs, + arrayNestedOptimizedFunctionsEnabled, }; } diff --git a/src/realm.js b/src/realm.js index 1bb20c781d..183157b511 100644 --- a/src/realm.js +++ b/src/realm.js @@ -337,6 +337,7 @@ export class Realm { this.debugNames = opts.debugNames; this._checkedObjectIds = new Map(); this.optimizedFunctions = new Map(); + this.arrayNestedOptimizedFunctionsEnabled = opts.arrayNestedOptimizedFunctionsEnabled || false; } statistics: RealmStatistics; @@ -482,6 +483,7 @@ export class Realm { _checkedObjectIds: Map<ObjectValue | AbstractObjectValue, number>; optimizedFunctions: Map<FunctionValue | AbstractValue, ArgModel | void>; + arrayNestedOptimizedFunctionsEnabled: boolean; // to force flow to type the annotations isCompatibleWith(compatibility: Compatibility): boolean { diff --git a/src/values/ArrayValue.js b/src/values/ArrayValue.js index c927c497d4..966e34a286 100644 --- a/src/values/ArrayValue.js +++ b/src/values/ArrayValue.js @@ -84,7 +84,7 @@ function createArrayWithWidenedNumericProperty( let abstractArrayValue = new ArrayValue(realm, intrinsicName); if (possibleNestedOptimizedFunctions !== undefined) { - if (!realm.react.enabled || realm.react.optimizeNestedFunctions) { + if (realm.arrayNestedOptimizedFunctionsEnabled && (!realm.react.enabled || realm.react.optimizeNestedFunctions)) { evaluatePossibleNestedOptimizedFunctionsAndStoreEffects( realm, abstractArrayValue, diff --git a/test/react/setupReactTests.js b/test/react/setupReactTests.js index 813ebd9e1b..d53c92ff48 100644 --- a/test/react/setupReactTests.js +++ b/test/react/setupReactTests.js @@ -133,6 +133,7 @@ ${source} reactEnabled: true, reactOutput: useJSXOutput ? "jsx" : "create-element", reactOptimizeNestedFunctions: true, + arrayNestedOptimizedFunctionsEnabled: true, inlineExpressions: true, invariantLevel: 0, stripFlow: true, From e233d31af7a69aeb1e4bca9d785b6857cf9a1253 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway <dg@domgan.com> Date: Thu, 9 Aug 2018 19:41:28 +0100 Subject: [PATCH 2/4] added explicit flag to tests --- scripts/test-runner.js | 5 ++++- .../additional-functions/NestedOptimizedFunction17.js | 2 ++ test/serializer/optimized-functions/ArrayFrom3.js | 2 ++ test/serializer/optimized-functions/ArrayFrom6.js | 1 + test/serializer/optimized-functions/ArrayFrom7.js | 1 + test/serializer/optimized-functions/ArrayFrom8.js | 1 + test/serializer/optimized-functions/ArrayFrom9.js | 1 + test/serializer/optimized-functions/Issue2359-1.js | 2 ++ test/serializer/optimized-functions/Issue2392-1.js | 2 ++ test/serializer/optimized-functions/Issue2392-2.js | 2 ++ 10 files changed, 18 insertions(+), 1 deletion(-) diff --git a/scripts/test-runner.js b/scripts/test-runner.js index 0af372b856..347c9ed9a6 100644 --- a/scripts/test-runner.js +++ b/scripts/test-runner.js @@ -363,8 +363,11 @@ function runTest(name, code, options: PrepackOptions, args) { internalDebug: true, serialize: true, uniqueSuffix: "", - arrayNestedOptimizedFunctionsEnabled: true, + arrayNestedOptimizedFunctionsEnabled: false, }): any): PrepackOptions); // Since PrepackOptions is an exact type I have to cast + if (code.includes("// arrayNestedOptimizedFunctionsEnabled")) { + options.arrayNestedOptimizedFunctionsEnabled = true; + } if (code.includes("// throws introspection error")) { try { let realmOptions = { diff --git a/test/serializer/additional-functions/NestedOptimizedFunction17.js b/test/serializer/additional-functions/NestedOptimizedFunction17.js index 31e17ed2ea..ef5d1703e4 100644 --- a/test/serializer/additional-functions/NestedOptimizedFunction17.js +++ b/test/serializer/additional-functions/NestedOptimizedFunction17.js @@ -1,3 +1,5 @@ +// arrayNestedOptimizedFunctionsEnabled + function fn(props, cond, cond2, cond3) { var arr = Array.from(props.x); var newObj; diff --git a/test/serializer/optimized-functions/ArrayFrom3.js b/test/serializer/optimized-functions/ArrayFrom3.js index 59ac4687e1..050c5f3a85 100644 --- a/test/serializer/optimized-functions/ArrayFrom3.js +++ b/test/serializer/optimized-functions/ArrayFrom3.js @@ -1,3 +1,5 @@ +// arrayNestedOptimizedFunctionsEnabled + function fn(x, y) { var edges = Array.from(x); var items = edges diff --git a/test/serializer/optimized-functions/ArrayFrom6.js b/test/serializer/optimized-functions/ArrayFrom6.js index 2aff88db9d..8c162275ef 100644 --- a/test/serializer/optimized-functions/ArrayFrom6.js +++ b/test/serializer/optimized-functions/ArrayFrom6.js @@ -1,4 +1,5 @@ // does not contain:// this function should be inlined +// arrayNestedOptimizedFunctionsEnabled (function() { function add(a, b) { diff --git a/test/serializer/optimized-functions/ArrayFrom7.js b/test/serializer/optimized-functions/ArrayFrom7.js index 131d7673a4..97f4f2dbba 100644 --- a/test/serializer/optimized-functions/ArrayFrom7.js +++ b/test/serializer/optimized-functions/ArrayFrom7.js @@ -1,4 +1,5 @@ // does not contain:// this function should be inlined +// arrayNestedOptimizedFunctionsEnabled (function() { var obj = { diff --git a/test/serializer/optimized-functions/ArrayFrom8.js b/test/serializer/optimized-functions/ArrayFrom8.js index be5d9c9a0a..3734ce3b08 100644 --- a/test/serializer/optimized-functions/ArrayFrom8.js +++ b/test/serializer/optimized-functions/ArrayFrom8.js @@ -1,4 +1,5 @@ // does contain:// this function should not be inlined +// arrayNestedOptimizedFunctionsEnabled (function() { var obj = { diff --git a/test/serializer/optimized-functions/ArrayFrom9.js b/test/serializer/optimized-functions/ArrayFrom9.js index 3da127908f..594b6d864c 100644 --- a/test/serializer/optimized-functions/ArrayFrom9.js +++ b/test/serializer/optimized-functions/ArrayFrom9.js @@ -1,4 +1,5 @@ // does contain:// this function should not be inlined +// arrayNestedOptimizedFunctionsEnabled (function() { function add(a, b) { diff --git a/test/serializer/optimized-functions/Issue2359-1.js b/test/serializer/optimized-functions/Issue2359-1.js index e7130d1c26..963b65f018 100644 --- a/test/serializer/optimized-functions/Issue2359-1.js +++ b/test/serializer/optimized-functions/Issue2359-1.js @@ -1,3 +1,5 @@ +// arrayNestedOptimizedFunctionsEnabled + function fn(x, cond, abstractFunc) { var arr = Array.from(x); diff --git a/test/serializer/optimized-functions/Issue2392-1.js b/test/serializer/optimized-functions/Issue2392-1.js index d9a1caf853..3f6bb6122b 100644 --- a/test/serializer/optimized-functions/Issue2392-1.js +++ b/test/serializer/optimized-functions/Issue2392-1.js @@ -1,3 +1,5 @@ +// arrayNestedOptimizedFunctionsEnabled + function fn(x, obj, cond, cond4) { var arr = Array.from(x); diff --git a/test/serializer/optimized-functions/Issue2392-2.js b/test/serializer/optimized-functions/Issue2392-2.js index e7fd82dac6..4ed3a86f57 100644 --- a/test/serializer/optimized-functions/Issue2392-2.js +++ b/test/serializer/optimized-functions/Issue2392-2.js @@ -1,3 +1,5 @@ +// arrayNestedOptimizedFunctionsEnabled + function fn(x, obj, cond, cond2, cond3, cond4) { var arr = Array.from(x); var a; From 983b93aec726e51ab53f94715407f5af9a7ce6dc Mon Sep 17 00:00:00 2001 From: Dominic Gannaway <dg@domgan.com> Date: Thu, 9 Aug 2018 20:42:30 +0100 Subject: [PATCH 3/4] add flag to test --- .../optimized-functions/DefineOptFuncInsideFuncInsideOptFunc.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/serializer/optimized-functions/DefineOptFuncInsideFuncInsideOptFunc.js b/test/serializer/optimized-functions/DefineOptFuncInsideFuncInsideOptFunc.js index e68b025340..40f78eebf6 100644 --- a/test/serializer/optimized-functions/DefineOptFuncInsideFuncInsideOptFunc.js +++ b/test/serializer/optimized-functions/DefineOptFuncInsideFuncInsideOptFunc.js @@ -1,3 +1,4 @@ +// arrayNestedOptimizedFunctionsEnabled // skip lint // The original issue here was that nested is defined inside of fn2 which is a non-optimized function // called by fn (an optimized function). That caused Prepack to not detect that nested was nested From fd6da583679663e589b5592b733d0623d0a2452e Mon Sep 17 00:00:00 2001 From: Dominic Gannaway <dg@domgan.com> Date: Fri, 10 Aug 2018 16:24:14 +0100 Subject: [PATCH 4/4] merge master and add flags --- test/serializer/optimized-functions/Issue2358.js | 2 ++ test/serializer/optimized-functions/Issue2398.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/test/serializer/optimized-functions/Issue2358.js b/test/serializer/optimized-functions/Issue2358.js index 838340c006..915c857497 100644 --- a/test/serializer/optimized-functions/Issue2358.js +++ b/test/serializer/optimized-functions/Issue2358.js @@ -1,3 +1,5 @@ +// arrayNestedOptimizedFunctionsEnabled + function fn(x, y, obj, cond) { var arr = Array.from(x); var arr2 = Array.from(y); diff --git a/test/serializer/optimized-functions/Issue2398.js b/test/serializer/optimized-functions/Issue2398.js index 6165bef7de..fbbf8ef6de 100644 --- a/test/serializer/optimized-functions/Issue2398.js +++ b/test/serializer/optimized-functions/Issue2398.js @@ -1,3 +1,5 @@ +// arrayNestedOptimizedFunctionsEnabled + function fn(props, cond) { var arr = Array.from(props.x); var newObj;