mirror of
https://github.com/facebookarchive/prepack.git
synced 2024-09-11 14:46:37 +03:00
Adds k limiting counter to simplification logic
Summary: Release notes: adds an abstract value limiting counter Following some discussion I had the other night with hermanventer in regards to having a counter for Prepack abstract logic computation, so rather than hang the compilation process, we could throw and recover using the pure logic. This PR adds a "k-limiting" counter to `AbstractValue`'s as a method `_checkAbstractValueImpliesCounter `. When we try and resolve abstract conditions on a single AbstractValue and we reach the limit, a `CompilerDiagnostic` along with a `FatalError` gets thrown. Locally this works well for the React compiler input. I only call `_checkAbstractValueImpliesCounter ` from `implies` right now and that might not be the best place, maybe it should be checked in other places too, but `implies` seemed the good place as it was in all stackframes when debugging. This also adds a new option called `abstractValueImpliesCounter ` which is based as an option to Realm. By default this feature is not turned on when the option `abstractValueImpliesCounter ` value is `0` (which it has been set to). Maybe we should enable this by default and set it to a sensible level? **Note:** I tried using the `timeout` feature, but it didn't work and wasn't too reliable in general because it would always fire in places where I didn't care for "time taken" but rather the depth on a certain feature of computation – in this case, abstract logic simplification. Enabling the timeout causes many other parts of compilation to break unless I explicitly set the timeout to `0` through all the React reconciliation work, which kind of defeats the purpose of it (the React reconciler work can take minutes to deal with on very large, expensive trees, and this is totally expected). Furthermore, we should avoid using non-deterministic features like `feature` in such cases as they can skew test results. Closes https://github.com/facebook/prepack/pull/1607 Differential Revision: D7312616 Pulled By: trueadm fbshipit-source-id: 8763df09b3182af4a496dec42d507aff1bb2cc8d
This commit is contained in:
parent
eb1449d6a1
commit
ba29f3c7e1
@ -48,6 +48,7 @@ let prepackOptions = {
|
||||
omitInvariants: true,
|
||||
abstractEffectsInAdditionalFunctions: true,
|
||||
simpleClosures: true,
|
||||
abstractValueImpliesMax: 1000,
|
||||
};
|
||||
let inputPath = path.resolve("fb-www/input.js");
|
||||
let outputPath = path.resolve("fb-www/output.js");
|
||||
|
@ -51,6 +51,7 @@ export type RealmOptions = {
|
||||
reactVerbose?: boolean,
|
||||
stripFlow?: boolean,
|
||||
abstractEffectsInAdditionalFunctions?: boolean,
|
||||
abstractValueImpliesMax?: number,
|
||||
};
|
||||
|
||||
export type SerializerOptions = {
|
||||
|
@ -55,6 +55,7 @@ export type PrepackOptions = {|
|
||||
maxStackDepth?: number,
|
||||
debugInFilePath?: string,
|
||||
debugOutFilePath?: string,
|
||||
abstractValueImpliesMax?: number,
|
||||
|};
|
||||
|
||||
export function getRealmOptions({
|
||||
@ -76,6 +77,7 @@ export function getRealmOptions({
|
||||
stripFlow,
|
||||
timeout,
|
||||
maxStackDepth,
|
||||
abstractValueImpliesMax,
|
||||
}: PrepackOptions): RealmOptions {
|
||||
return {
|
||||
abstractEffectsInAdditionalFunctions,
|
||||
@ -96,6 +98,7 @@ export function getRealmOptions({
|
||||
stripFlow,
|
||||
timeout,
|
||||
maxStackDepth,
|
||||
abstractValueImpliesMax,
|
||||
};
|
||||
}
|
||||
|
||||
|
@ -160,6 +160,8 @@ export class Realm {
|
||||
}
|
||||
this.strictlyMonotonicDateNow = !!opts.strictlyMonotonicDateNow;
|
||||
|
||||
// 0 = disabled
|
||||
this.abstractValueImpliesMax = opts.abstractValueImpliesMax || 0;
|
||||
this.timeout = opts.timeout;
|
||||
if (this.timeout) {
|
||||
// We'll call Date.now for every this.timeoutCounterThreshold'th AST node.
|
||||
@ -228,6 +230,7 @@ export class Realm {
|
||||
trackLeaks: boolean;
|
||||
debugNames: void | boolean;
|
||||
isInPureTryStatement: boolean; // TODO(1264): Remove this once we implement proper exception handling in abstract calls.
|
||||
abstractValueImpliesMax: number;
|
||||
timeout: void | number;
|
||||
mathRandomGenerator: void | (() => number);
|
||||
strictlyMonotonicDateNow: boolean;
|
||||
|
@ -31,6 +31,9 @@ export default function simplifyAndRefineAbstractValue(
|
||||
};
|
||||
return simplify(realm, value, isCondition);
|
||||
} catch (e) {
|
||||
if (e instanceof FatalError) {
|
||||
throw e;
|
||||
}
|
||||
return value;
|
||||
} finally {
|
||||
realm.errorHandler = savedHandler;
|
||||
|
@ -17,7 +17,7 @@ import type {
|
||||
BabelNodeSourceLocation,
|
||||
BabelUnaryOperator,
|
||||
} from "babel-types";
|
||||
import { FatalError } from "../errors.js";
|
||||
import { FatalError, CompilerDiagnostic } from "../errors.js";
|
||||
import type { Realm } from "../realm.js";
|
||||
import type { PropertyKeyValue } from "../types.js";
|
||||
import { PreludeGenerator } from "../utils/generator.js";
|
||||
@ -66,6 +66,7 @@ export default class AbstractValue extends Value {
|
||||
this.args = args;
|
||||
this.hashValue = hashValue;
|
||||
this.kind = optionalArgs ? optionalArgs.kind : undefined;
|
||||
this.impliesCounter = 0;
|
||||
}
|
||||
|
||||
hashValue: number;
|
||||
@ -75,6 +76,7 @@ export default class AbstractValue extends Value {
|
||||
mightBeEmpty: boolean;
|
||||
args: Array<Value>;
|
||||
_buildNode: void | AbstractValueBuildNodeFunction | BabelNodeExpression;
|
||||
impliesCounter: number;
|
||||
|
||||
addSourceLocationsTo(locations: Array<BabelNodeSourceLocation>, seenValues?: Set<AbstractValue> = new Set()) {
|
||||
if (seenValues.has(this)) return;
|
||||
@ -174,8 +176,26 @@ export default class AbstractValue extends Value {
|
||||
return this._buildNode && this._buildNode.type === "Identifier";
|
||||
}
|
||||
|
||||
_checkAbstractValueImpliesCounter() {
|
||||
let realm = this.$Realm;
|
||||
let abstractValueImpliesMax = realm.abstractValueImpliesMax;
|
||||
// if abstractValueImpliesMax is 0, then the counter is disabled
|
||||
if (abstractValueImpliesMax !== 0 && this.impliesCounter++ > abstractValueImpliesMax) {
|
||||
this.impliesCounter = 0;
|
||||
let diagnostic = new CompilerDiagnostic(
|
||||
`the implies counter has exceeded the maximum value when trying to simplify abstract values`,
|
||||
realm.currentLocation,
|
||||
"PP0029",
|
||||
"FatalError"
|
||||
);
|
||||
realm.handleError(diagnostic);
|
||||
if (realm.handleError(diagnostic) === "Fail") throw new FatalError();
|
||||
}
|
||||
}
|
||||
|
||||
// this => val. A false value does not imply that !(this => val).
|
||||
implies(val: Value): boolean {
|
||||
this._checkAbstractValueImpliesCounter();
|
||||
if (this.equals(val)) return true; // x => x regardless of its value
|
||||
if (!this.mightNotBeFalse()) return true; // false => val
|
||||
if (!val.mightNotBeTrue()) return true; // x => true regardless of the value of x
|
||||
|
Loading…
Reference in New Issue
Block a user