mirror of
https://github.com/facebookarchive/prepack.git
synced 2024-08-16 18:10:33 +03:00
Add abstract serializer mode for test262 execution (#2297)
Summary: I extended the `--serializer` command line argument I added in #2290 to now support `--serializer abstract-scalar`. What this mode does is it converts all boolean, string, number, and symbol literals into abstract values. I did not choose to extend this logic to object and array literals just yet since scalars alone showed some interesting results. What I really want here is a review of the results. Full suite execution results are real bad. **18%** pass rate. I dug a bit into why. ``` === RESULTS === Passes: 3356 / 17780 (18%) ES5 passes: 2276 / 12045 (18%) ES6 passes: 1080 / 5735 (18%) Skipped: 13375 Timeouts: 28 ``` I was mostly interested in the runtime failures we see since that means Prepack is serializing invalid code. However, I found ~14k failures in the Prepack stage (more on this in a bit) and ~3k failures in the runtime stage. This means ~80% of tests _fail to compile_ with this abstract transformation applied. Why are these tests failing? I took the first 4 items of the stack traces from errors thrown in the Prepack stage, sorted, and ranked them. [Here’s the result.](https://gist.github.com/calebmer/29e27613325fd99fa04be7ab4a9641c0) The top 5 with thousands of hits are: ``` 7538 of: at AbstractValue.throwIfNotConcrete (/Users/calebmer/prepack/src/values/AbstractValue.js:536:11) at ToImplementation.ToStringPartial (/Users/calebmer/prepack/src/methods/to.js:717:69) at NativeFunctionValue._index.NativeFunctionValue [as callback] (/Users/calebmer/prepack/src/intrinsics/ecma262/String.js:34:37) at NativeFunctionValue.callCallback (/Users/calebmer/prepack/src/values/NativeFunctionValue.js:121:12) 4595 of: at AbstractValue.throwIfNotConcrete (/Users/calebmer/prepack/src/values/AbstractValue.js:536:11) at NativeFunctionValue.func.defineNativeMethod [as callback] (/Users/calebmer/prepack/src/intrinsics/ecma262/Object.js:328:41) at NativeFunctionValue.callCallback (/Users/calebmer/prepack/src/values/NativeFunctionValue.js:121:12) at functionCall (/Users/calebmer/prepack/src/methods/call.js:308:26) 1454 of: at AbstractValue.throwIfNotConcrete (/Users/calebmer/prepack/src/values/AbstractValue.js:536:11) at NativeFunctionValue.func.defineNativeMethod [as callback] (/Users/calebmer/prepack/src/intrinsics/ecma262/Object.js:364:41) at NativeFunctionValue.callCallback (/Users/calebmer/prepack/src/values/NativeFunctionValue.js:121:12) at functionCall (/Users/calebmer/prepack/src/methods/call.js:308:26) 1351 of: at invariant (/Users/calebmer/prepack/src/invariant.js:18:15) at EvalPropertyNamePartial (/Users/calebmer/prepack/src/evaluators/ObjectExpression.js:59:7) at _default (/Users/calebmer/prepack/src/evaluators/ObjectExpression.js:80:21) at LexicalEnvironment.evaluateAbstract (/Users/calebmer/prepack/src/environment.js:1368:20) 1053 of: at AbstractValue.throwIfNotConcrete (/Users/calebmer/prepack/src/values/AbstractValue.js:536:11) at NativeFunctionValue.obj.defineNativeMethod [as callback] (/Users/calebmer/prepack/src/intrinsics/ecma262/ObjectPrototype.js:35:39) at NativeFunctionValue.callCallback (/Users/calebmer/prepack/src/values/NativeFunctionValue.js:121:12) at functionCall (/Users/calebmer/prepack/src/methods/call.js:308:26) ``` This means there may be some low hanging fruit. Here are my questions for you. - Did you expect results like this? - What is our ideal test262 pass rate with this transformation applied? - What happens to React Compiler or other projects when these errors are thrown? (As I understand it, we bail out and don’t optimize the code, but do optimize the code around it.) - Do you think my methodology is flawed? It’s also possible that something in my methodology is wrong, but I didn’t spend much time investigating these failures as I spent investigating the failures I found in #2290. My goal with this test suite is to build an understanding of what “correctness” for the React Compiler against all JavaScript code looks like. (Not just the few bundles we’ve selected to look at.) I don’t think these results suggest that we only safely compile 18% of the language, but it’s a data point. I’ll be looking into fixing a selection of these issues to better understand their nature or if I need to change methodologies. Pull Request resolved: https://github.com/facebook/prepack/pull/2297 Differential Revision: D9120572 Pulled By: calebmer fbshipit-source-id: b394f1e8da034c9985366010e3e63fd55fd94168
This commit is contained in:
parent
f783183960
commit
88d9495226
@ -33,6 +33,9 @@ import tty from "tty";
|
|||||||
import minimist from "minimist";
|
import minimist from "minimist";
|
||||||
import process from "process";
|
import process from "process";
|
||||||
import vm from "vm";
|
import vm from "vm";
|
||||||
|
import * as babelTypes from "@babel/types";
|
||||||
|
import traverse from "@babel/traverse";
|
||||||
|
import generate from "@babel/generator";
|
||||||
|
|
||||||
const EOL = os.EOL;
|
const EOL = os.EOL;
|
||||||
const cpus = os.cpus();
|
const cpus = os.cpus();
|
||||||
@ -45,7 +48,7 @@ type GroupsMap = { [key: string]: TestRecord[] };
|
|||||||
|
|
||||||
type TestRunOptions = {|
|
type TestRunOptions = {|
|
||||||
+timeout: number,
|
+timeout: number,
|
||||||
+serializer: boolean,
|
+serializer: boolean | "abstract-scalar",
|
||||||
|};
|
|};
|
||||||
|
|
||||||
// A TestTask is a task for a worker process to execute, which contains a
|
// A TestTask is a task for a worker process to execute, which contains a
|
||||||
@ -242,7 +245,7 @@ class MasterProgramArgs {
|
|||||||
filterString: string;
|
filterString: string;
|
||||||
singleThreaded: boolean;
|
singleThreaded: boolean;
|
||||||
relativeTestPath: string;
|
relativeTestPath: string;
|
||||||
serializer: boolean;
|
serializer: boolean | "abstract-scalar";
|
||||||
expectedES5: number;
|
expectedES5: number;
|
||||||
expectedES6: number;
|
expectedES6: number;
|
||||||
expectedTimeouts: number;
|
expectedTimeouts: number;
|
||||||
@ -256,7 +259,7 @@ class MasterProgramArgs {
|
|||||||
filterString: string,
|
filterString: string,
|
||||||
singleThreaded: boolean,
|
singleThreaded: boolean,
|
||||||
relativeTestPath: string,
|
relativeTestPath: string,
|
||||||
serializer: boolean,
|
serializer: boolean | "abstract-scalar",
|
||||||
expectedES5: number,
|
expectedES5: number,
|
||||||
expectedES6: number,
|
expectedES6: number,
|
||||||
expectedTimeouts: number
|
expectedTimeouts: number
|
||||||
@ -279,9 +282,9 @@ class MasterProgramArgs {
|
|||||||
class WorkerProgramArgs {
|
class WorkerProgramArgs {
|
||||||
relativeTestPath: string;
|
relativeTestPath: string;
|
||||||
timeout: number;
|
timeout: number;
|
||||||
serializer: boolean;
|
serializer: boolean | "abstract-scalar";
|
||||||
|
|
||||||
constructor(relativeTestPath: string, timeout: number, serializer: boolean) {
|
constructor(relativeTestPath: string, timeout: number, serializer: boolean | "abstract-scalar") {
|
||||||
this.timeout = timeout;
|
this.timeout = timeout;
|
||||||
this.serializer = serializer;
|
this.serializer = serializer;
|
||||||
this.relativeTestPath = relativeTestPath;
|
this.relativeTestPath = relativeTestPath;
|
||||||
@ -353,7 +356,7 @@ function usage(): string {
|
|||||||
function masterArgsParse(): MasterProgramArgs {
|
function masterArgsParse(): MasterProgramArgs {
|
||||||
let parsedArgs = minimist(process.argv.slice(2), {
|
let parsedArgs = minimist(process.argv.slice(2), {
|
||||||
string: ["statusFile", "relativeTestPath"],
|
string: ["statusFile", "relativeTestPath"],
|
||||||
boolean: ["verbose", "singleThreaded", "serializer"],
|
boolean: ["verbose", "singleThreaded"],
|
||||||
default: {
|
default: {
|
||||||
verbose: process.stdout instanceof tty.WriteStream ? false : true,
|
verbose: process.stdout instanceof tty.WriteStream ? false : true,
|
||||||
statusFile: "",
|
statusFile: "",
|
||||||
@ -395,8 +398,10 @@ function masterArgsParse(): MasterProgramArgs {
|
|||||||
throw new ArgsParseError("relativeTestPath must be a string (--relativeTestPath /../test/test262)");
|
throw new ArgsParseError("relativeTestPath must be a string (--relativeTestPath /../test/test262)");
|
||||||
}
|
}
|
||||||
let relativeTestPath = parsedArgs.relativeTestPath;
|
let relativeTestPath = parsedArgs.relativeTestPath;
|
||||||
if (typeof parsedArgs.serializer !== "boolean") {
|
if (!(typeof parsedArgs.serializer === "boolean" || parsedArgs.serializer === "abstract-scalar")) {
|
||||||
throw new ArgsParseError("serializer must be a boolean (either --serializer or not)");
|
throw new ArgsParseError(
|
||||||
|
"serializer must be a boolean or must be the string 'abstract-scalar' (--serializer or --serializer abstract-scalar)"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
let serializer = parsedArgs.serializer;
|
let serializer = parsedArgs.serializer;
|
||||||
if (typeof parsedArgs.expectedCounts !== "string") {
|
if (typeof parsedArgs.expectedCounts !== "string") {
|
||||||
@ -438,8 +443,10 @@ function workerArgsParse(): WorkerProgramArgs {
|
|||||||
if (typeof parsedArgs.timeout !== "number") {
|
if (typeof parsedArgs.timeout !== "number") {
|
||||||
throw new ArgsParseError("timeout must be a number (in seconds) (--timeout 10)");
|
throw new ArgsParseError("timeout must be a number (in seconds) (--timeout 10)");
|
||||||
}
|
}
|
||||||
if (typeof parsedArgs.serializer !== "boolean") {
|
if (!(typeof parsedArgs.serializer === "boolean" || parsedArgs.serializer === "abstract-scalar")) {
|
||||||
throw new ArgsParseError("serializer must be a boolean (either --serializer or not)");
|
throw new ArgsParseError(
|
||||||
|
"serializer must be a boolean or must be the string 'abstract-scalar' (--serializer or --serializer abstract-scalar)"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
return new WorkerProgramArgs(parsedArgs.relativeTestPath, parsedArgs.timeout, parsedArgs.serializer);
|
return new WorkerProgramArgs(parsedArgs.relativeTestPath, parsedArgs.timeout, parsedArgs.serializer);
|
||||||
}
|
}
|
||||||
@ -1023,13 +1030,13 @@ function runTest(
|
|||||||
// eslint-disable-next-line flowtype/no-weak-types
|
// eslint-disable-next-line flowtype/no-weak-types
|
||||||
harnesses: Object,
|
harnesses: Object,
|
||||||
strict: boolean,
|
strict: boolean,
|
||||||
{ timeout, serializer }: TestRunOptions
|
options: TestRunOptions
|
||||||
): ?TestResult {
|
): ?TestResult {
|
||||||
if (serializer) {
|
if (options.serializer) {
|
||||||
return executeTestUsingSerializer(test, testFileContents, data, harnesses, strict, timeout);
|
return executeTestUsingSerializer(test, testFileContents, data, harnesses, strict, options);
|
||||||
}
|
}
|
||||||
|
|
||||||
let { realm } = createRealm(timeout);
|
let { realm } = createRealm(options.timeout);
|
||||||
|
|
||||||
// Run the test.
|
// Run the test.
|
||||||
try {
|
try {
|
||||||
@ -1136,8 +1143,9 @@ function executeTestUsingSerializer(
|
|||||||
// eslint-disable-next-line flowtype/no-weak-types
|
// eslint-disable-next-line flowtype/no-weak-types
|
||||||
harnesses: Object,
|
harnesses: Object,
|
||||||
strict: boolean,
|
strict: boolean,
|
||||||
timeout: number
|
options: TestRunOptions
|
||||||
) {
|
) {
|
||||||
|
let { timeout } = options;
|
||||||
let sources = [];
|
let sources = [];
|
||||||
|
|
||||||
// Add the test262 intrinsics.
|
// Add the test262 intrinsics.
|
||||||
@ -1172,13 +1180,38 @@ var print = () => {}; // noop for now
|
|||||||
if (diag.severity !== "Warning") return "Fail";
|
if (diag.severity !== "Warning") return "Fail";
|
||||||
return "Recover";
|
return "Recover";
|
||||||
},
|
},
|
||||||
onParse: () => {
|
onParse: ast => {
|
||||||
// TODO(calebmer): Turn all literals into abstract values
|
// Transform all statements which come from our test source file. Do not transform statements from our
|
||||||
|
// harness files.
|
||||||
|
if (options.serializer === "abstract-scalar") {
|
||||||
|
ast.program.body.forEach(node => {
|
||||||
|
if ((node.loc: any).filename === test.location) {
|
||||||
|
transformScalarsToAbstractValues(node);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (error.message === "Timed out") return new TestResult(false, strict, error);
|
if (error.message === "Timed out") return new TestResult(false, strict, error);
|
||||||
if (error.message.includes("Syntax error")) return null;
|
if (error.message.includes("Syntax error")) return null;
|
||||||
|
// Uncomment the following JS code to do analysis on what kinds of Prepack errors we get.
|
||||||
|
//
|
||||||
|
// ```js
|
||||||
|
// console.error(
|
||||||
|
// `${error.name.replace(/\n/g, "\\n")}: ${error.message.replace(/\n/g, "\\n")} (${error.stack
|
||||||
|
// .match(/at .+$/gm)
|
||||||
|
// .slice(0, 3)
|
||||||
|
// .join(", ")})`
|
||||||
|
// );
|
||||||
|
// ```
|
||||||
|
//
|
||||||
|
// Analysis bash command:
|
||||||
|
//
|
||||||
|
// ```bash
|
||||||
|
// yarn test-test262 --serializer 2> result.err
|
||||||
|
// cat result.err | sort | uniq -c | sort -nr
|
||||||
|
// ```
|
||||||
return new TestResult(false, strict, new Error(`Prepack error:\n${error.stack}`));
|
return new TestResult(false, strict, new Error(`Prepack error:\n${error.stack}`));
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1205,6 +1238,83 @@ var print = () => {}; // noop for now
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const TransformScalarsToAbstractValuesVisitor = (() => {
|
||||||
|
const t = babelTypes;
|
||||||
|
|
||||||
|
function createAbstractCall(type, actual, { allowDuplicateNames, disablePlaceholders } = {}) {
|
||||||
|
const args = [type, actual];
|
||||||
|
if (allowDuplicateNames) {
|
||||||
|
args.push(
|
||||||
|
t.objectExpression([
|
||||||
|
t.objectProperty(t.identifier("allowDuplicateNames"), t.booleanLiteral(!!allowDuplicateNames)),
|
||||||
|
t.objectProperty(t.identifier("disablePlaceholders"), t.booleanLiteral(!!disablePlaceholders)),
|
||||||
|
])
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return t.callExpression(t.identifier("__abstract"), args);
|
||||||
|
}
|
||||||
|
|
||||||
|
const defaultOptions = {
|
||||||
|
allowDuplicateNames: true,
|
||||||
|
disablePlaceholders: true,
|
||||||
|
};
|
||||||
|
|
||||||
|
const symbolOptions = {
|
||||||
|
// Intentionally false since two symbol calls will be referentially not equal, but Prepack will share
|
||||||
|
// a variable.
|
||||||
|
allowDuplicateNames: false,
|
||||||
|
disablePlaceholders: true,
|
||||||
|
};
|
||||||
|
|
||||||
|
return {
|
||||||
|
noScope: true,
|
||||||
|
|
||||||
|
BooleanLiteral(p) {
|
||||||
|
p.node = p.container[p.key] = createAbstractCall(
|
||||||
|
t.stringLiteral("boolean"),
|
||||||
|
t.stringLiteral(p.node.value.toString()),
|
||||||
|
defaultOptions
|
||||||
|
);
|
||||||
|
},
|
||||||
|
StringLiteral(p) {
|
||||||
|
// `eval()` does not support abstract arguments and we don't care to fix that.
|
||||||
|
if (
|
||||||
|
p.parent.type === "CallExpression" &&
|
||||||
|
p.parent.callee.type === "Identifier" &&
|
||||||
|
p.parent.callee.name === "eval"
|
||||||
|
) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
p.node = p.container[p.key] = createAbstractCall(
|
||||||
|
t.stringLiteral("string"),
|
||||||
|
t.stringLiteral(JSON.stringify(p.node.value)),
|
||||||
|
defaultOptions
|
||||||
|
);
|
||||||
|
},
|
||||||
|
CallExpression(p) {
|
||||||
|
if (p.node.callee.type === "Identifier" && p.node.callee.name === "Symbol") {
|
||||||
|
p.node = p.container[p.key] = createAbstractCall(
|
||||||
|
t.stringLiteral("symbol"),
|
||||||
|
t.stringLiteral(generate(p.node).code),
|
||||||
|
symbolOptions
|
||||||
|
);
|
||||||
|
}
|
||||||
|
},
|
||||||
|
NumericLiteral(p) {
|
||||||
|
p.node = p.container[p.key] = createAbstractCall(
|
||||||
|
t.stringLiteral(Number.isInteger(p.node.value) ? "integral" : "number"),
|
||||||
|
t.stringLiteral(p.node.extra.raw),
|
||||||
|
defaultOptions
|
||||||
|
);
|
||||||
|
},
|
||||||
|
};
|
||||||
|
})();
|
||||||
|
|
||||||
|
function transformScalarsToAbstractValues(ast) {
|
||||||
|
traverse(ast, TransformScalarsToAbstractValuesVisitor);
|
||||||
|
traverse.cache.clear();
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns true if ${test} should be run, false otherwise
|
* Returns true if ${test} should be run, false otherwise
|
||||||
*/
|
*/
|
||||||
|
@ -80,6 +80,8 @@ export default function(realm: Realm): void {
|
|||||||
// options is an optional object that may contain:
|
// options is an optional object that may contain:
|
||||||
// - allowDuplicateNames: boolean representing whether the name of the abstract value may be
|
// - allowDuplicateNames: boolean representing whether the name of the abstract value may be
|
||||||
// repeated, by default they must be unique
|
// repeated, by default they must be unique
|
||||||
|
// - disablePlaceholders: boolean representing whether placeholders should be substituted in
|
||||||
|
// the abstract value's name.
|
||||||
// If the abstract value gets somehow embedded in the final heap,
|
// If the abstract value gets somehow embedded in the final heap,
|
||||||
// it will be referred to by the supplied name in the generated code.
|
// it will be referred to by the supplied name in the generated code.
|
||||||
global.$DefineOwnProperty("__abstract", {
|
global.$DefineOwnProperty("__abstract", {
|
||||||
|
@ -104,7 +104,13 @@ export function createAbstract(
|
|||||||
} else {
|
} else {
|
||||||
realm.saveNameString(name);
|
realm.saveNameString(name);
|
||||||
}
|
}
|
||||||
result = AbstractValue.createFromTemplate(realm, buildExpressionTemplate(name), type, [], kind);
|
result = AbstractValue.createFromTemplate(
|
||||||
|
realm,
|
||||||
|
buildExpressionTemplate(name, { disablePlaceholders: !!optionsMap.get("disablePlaceholders") }),
|
||||||
|
type,
|
||||||
|
[],
|
||||||
|
kind
|
||||||
|
);
|
||||||
result.intrinsicName = name;
|
result.intrinsicName = name;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -15,12 +15,20 @@ import type { PreludeGenerator } from "./PreludeGenerator.js";
|
|||||||
import invariant from "../invariant.js";
|
import invariant from "../invariant.js";
|
||||||
|
|
||||||
const placeholders = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".split("");
|
const placeholders = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".split("");
|
||||||
const placeholderWhitelist = new Set(["global", ...placeholders]);
|
const placeholderDefaultWhiteList = new Set(["global"]);
|
||||||
|
const placeholderWhitelist = new Set([...placeholderDefaultWhiteList, ...placeholders]);
|
||||||
|
|
||||||
export default function buildExpressionTemplate(code: string): (void | PreludeGenerator) => any => BabelNodeExpression {
|
export default function buildExpressionTemplate(
|
||||||
|
code: string,
|
||||||
|
{ disablePlaceholders }: { disablePlaceholders?: boolean } = {}
|
||||||
|
): (void | PreludeGenerator) => any => BabelNodeExpression {
|
||||||
let template;
|
let template;
|
||||||
return (preludeGenerator: void | PreludeGenerator) => (obj: any): BabelNodeExpression => {
|
return (preludeGenerator: void | PreludeGenerator) => (obj: any): BabelNodeExpression => {
|
||||||
if (template === undefined) template = buildTemplate(code, { placeholderPattern: false, placeholderWhitelist });
|
if (template === undefined)
|
||||||
|
template = buildTemplate(code, {
|
||||||
|
placeholderPattern: false,
|
||||||
|
placeholderWhitelist: disablePlaceholders ? placeholderDefaultWhiteList : placeholderWhitelist,
|
||||||
|
});
|
||||||
if (preludeGenerator !== undefined && code.includes("global"))
|
if (preludeGenerator !== undefined && code.includes("global"))
|
||||||
obj = Object.assign(
|
obj = Object.assign(
|
||||||
{
|
{
|
||||||
|
Loading…
Reference in New Issue
Block a user