Skip to content

Commit

Permalink
Improve control flow type caching (#35332)
Browse files Browse the repository at this point in the history
* Cache getTypeOfExpression results when CFA was invoked

* Add tests

* Accept new baselines
  • Loading branch information
ahejlsberg authored Nov 25, 2019
1 parent 0c2c58c commit ea4e6b3
Show file tree
Hide file tree
Showing 6 changed files with 1,572 additions and 26 deletions.
63 changes: 39 additions & 24 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,7 @@ namespace ts {
let flowInvocationCount = 0;
let lastFlowNode: FlowNode | undefined;
let lastFlowNodeReachable: boolean;
let flowTypeCache: Type[] | undefined;

const emptyStringType = getLiteralType("");
const zeroType = getLiteralType(0);
Expand All @@ -844,7 +845,6 @@ namespace ts {
const symbolLinks: SymbolLinks[] = [];
const nodeLinks: NodeLinks[] = [];
const flowLoopCaches: Map<Type>[] = [];
const flowAssignmentTypes: Type[] = [];
const flowLoopNodes: FlowNode[] = [];
const flowLoopKeys: string[] = [];
const flowLoopTypes: Type[][] = [];
Expand Down Expand Up @@ -19173,23 +19173,9 @@ namespace ts {

function getInitialOrAssignedType(flow: FlowAssignment) {
const node = flow.node;
if (flow.flags & FlowFlags.Cached) {
const cached = flowAssignmentTypes[getNodeId(node)];
if (cached) {
return cached;
}
}
const startInvocationCount = flowInvocationCount;
const type = getConstraintForLocation(node.kind === SyntaxKind.VariableDeclaration || node.kind === SyntaxKind.BindingElement ?
return getConstraintForLocation(node.kind === SyntaxKind.VariableDeclaration || node.kind === SyntaxKind.BindingElement ?
getInitialType(<VariableDeclaration | BindingElement>node) :
getAssignedType(node), reference);
// We cache the assigned type when getFlowTypeOfReference was recursively invoked in the
// resolution of the assigned type and we're not within loop analysis.
if (flowInvocationCount !== startInvocationCount && flowLoopCount === flowLoopStart) {
flow.flags |= FlowFlags.Cached;
flowAssignmentTypes[getNodeId(node)] = type;
}
return type;
}

function getTypeAtFlowAssignment(flow: FlowAssignment) {
Expand Down Expand Up @@ -19469,7 +19455,10 @@ namespace ts {
flowLoopKeys[flowLoopCount] = key;
flowLoopTypes[flowLoopCount] = antecedentTypes;
flowLoopCount++;
const saveFlowTypeCache = flowTypeCache;
flowTypeCache = undefined;
flowType = getTypeAtFlowNode(antecedent);
flowTypeCache = saveFlowTypeCache;
flowLoopCount--;
// If we see a value appear in the cache it is a sign that control flow analysis
// was restarted and completed by checkExpressionCached. We can simply pick up
Expand Down Expand Up @@ -27322,8 +27311,11 @@ namespace ts {
// analysis because variables may have transient types in indeterminable states. Moving flowLoopStart
// to the top of the stack ensures all transient types are computed from a known point.
const saveFlowLoopStart = flowLoopStart;
const saveFlowTypeCache = flowTypeCache;
flowLoopStart = flowLoopCount;
flowTypeCache = undefined;
links.resolvedType = checkExpression(node, checkMode);
flowTypeCache = saveFlowTypeCache;
flowLoopStart = saveFlowLoopStart;
}
return links.resolvedType;
Expand All @@ -27336,7 +27328,7 @@ namespace ts {

function checkDeclarationInitializer(declaration: HasExpressionInitializer) {
const initializer = getEffectiveInitializer(declaration)!;
const type = getTypeOfExpression(initializer, /*cache*/ true);
const type = getQuickTypeOfExpression(initializer) || checkExpressionCached(initializer);
const padded = isParameter(declaration) && declaration.name.kind === SyntaxKind.ArrayBindingPattern &&
isTupleType(type) && !type.target.hasRestElement && getTypeReferenceArity(type) < declaration.name.elements.length ?
padTupleType(type, declaration.name) : type;
Expand Down Expand Up @@ -27590,10 +27582,32 @@ namespace ts {
/**
* Returns the type of an expression. Unlike checkExpression, this function is simply concerned
* with computing the type and may not fully check all contained sub-expressions for errors.
* A cache argument of true indicates that if the function performs a full type check, it is ok
* to cache the result.
*/
function getTypeOfExpression(node: Expression, cache?: boolean) {
function getTypeOfExpression(node: Expression) {
// Don't bother caching types that require no flow analysis and are quick to compute.
const quickType = getQuickTypeOfExpression(node);
if (quickType) {
return quickType;
}
// If a type has been cached for the node, return it.
if (node.flags & NodeFlags.TypeCached && flowTypeCache) {
const cachedType = flowTypeCache[getNodeId(node)];
if (cachedType) {
return cachedType;
}
}
const startInvocationCount = flowInvocationCount;
const type = checkExpression(node);
// If control flow analysis was required to determine the type, it is worth caching.
if (flowInvocationCount !== startInvocationCount) {
const cache = flowTypeCache || (flowTypeCache = []);
cache[getNodeId(node)] = type;
node.flags |= NodeFlags.TypeCached;
}
return type;
}

function getQuickTypeOfExpression(node: Expression) {
const expr = skipParentheses(node);
// Optimize for the common case of a call to a function with a single non-generic call
// signature where we can just fetch the return type without checking the arguments.
Expand All @@ -27607,10 +27621,11 @@ namespace ts {
else if (isAssertionExpression(expr) && !isConstTypeReference(expr.type)) {
return getTypeFromTypeNode((<TypeAssertion>expr).type);
}
// Otherwise simply call checkExpression. Ideally, the entire family of checkXXX functions
// should have a parameter that indicates whether full error checking is required such that
// we can perform the optimizations locally.
return cache ? checkExpressionCached(node) : checkExpression(node);
else if (node.kind === SyntaxKind.NumericLiteral || node.kind === SyntaxKind.StringLiteral ||
node.kind === SyntaxKind.TrueKeyword || node.kind === SyntaxKind.FalseKeyword) {
return checkExpression(node);
}
return undefined;
}

/**
Expand Down
3 changes: 1 addition & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ namespace ts {
/* @internal */ Ambient = 1 << 23, // If node was inside an ambient context -- a declaration file, or inside something with the `declare` modifier.
/* @internal */ InWithStatement = 1 << 24, // If any ancestor of node was the `statement` of a WithStatement (not the `expression`)
JsonFile = 1 << 25, // If node was parsed in a Json
/* @internal */ TypeCached = 1 << 26, // If a type was cached for node at any point

BlockScoped = Let | Const,

Expand Down Expand Up @@ -2699,8 +2700,6 @@ namespace ts {
Shared = 1 << 11, // Referenced as antecedent more than once
PreFinally = 1 << 12, // Injected edge that links pre-finally label and pre-try flow
AfterFinally = 1 << 13, // Injected edge that links post-finally flow with the rest of the graph
/** @internal */
Cached = 1 << 14, // Indicates that at least one cross-call cache entry exists for this node, even if not a loop participant

Label = BranchLabel | LoopLabel,
Condition = TrueCondition | FalseCondition,
Expand Down
Loading

0 comments on commit ea4e6b3

Please sign in to comment.