Skip to content

Improve caching in recursive type comparisons (fixes #1170) #1180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 17, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 25 additions & 12 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3221,9 +3221,9 @@ module ts {

// TYPE CHECKING

var subtypeRelation: Map<Ternary> = {};
var assignableRelation: Map<Ternary> = {};
var identityRelation: Map<Ternary> = {};
var subtypeRelation: Map<boolean> = {};
var assignableRelation: Map<boolean> = {};
var identityRelation: Map<boolean> = {};

function isTypeIdenticalTo(source: Type, target: Type): boolean {
return checkTypeRelatedTo(source, target, identityRelation, /*errorNode*/ undefined);
Expand Down Expand Up @@ -3258,14 +3258,15 @@ module ts {
function checkTypeRelatedTo(
source: Type,
target: Type,
relation: Map<Ternary>,
relation: Map<boolean>,
errorNode: Node,
headMessage?: DiagnosticMessage,
containingMessageChain?: DiagnosticMessageChain): boolean {

var errorInfo: DiagnosticMessageChain;
var sourceStack: ObjectType[];
var targetStack: ObjectType[];
var maybeStack: Map<boolean>[];
var expandingFlags: number;
var depth = 0;
var overflow = false;
Expand Down Expand Up @@ -3424,12 +3425,12 @@ module ts {
var id = source.id + "," + target.id;
var related = relation[id];
if (related !== undefined) {
return related;
return related ? Ternary.True : Ternary.False;
}
if (depth > 0) {
for (var i = 0; i < depth; i++) {
// If source and target are already being compared, consider them related with assumptions
if (source === sourceStack[i] && target === targetStack[i]) {
if (maybeStack[i][id]) {
return Ternary.Maybe;
}
}
Expand All @@ -3441,16 +3442,19 @@ module ts {
else {
sourceStack = [];
targetStack = [];
maybeStack = [];
expandingFlags = 0;
}
sourceStack[depth] = source;
targetStack[depth] = target;
maybeStack[depth] = {};
maybeStack[depth][id] = true;
depth++;
var saveExpandingFlags = expandingFlags;
if (!(expandingFlags & 1) && isDeeplyNestedGeneric(source, sourceStack)) expandingFlags |= 1;
if (!(expandingFlags & 2) && isDeeplyNestedGeneric(target, targetStack)) expandingFlags |= 2;
if (expandingFlags === 3) {
var result = Ternary.True;
var result = Ternary.Maybe;
}
else {
var result = propertiesRelatedTo(source, target, reportErrors);
Expand All @@ -3469,9 +3473,18 @@ module ts {
}
expandingFlags = saveExpandingFlags;
depth--;
// Only cache results that are free of assumptions
if (result !== Ternary.Maybe) {
relation[id] = result;
if (result) {
var maybeCache = maybeStack[depth];
// If result is definitely true, copy assumptions to global cache, else copy to next level up
var destinationCache = result === Ternary.True || depth === 0 ? relation : maybeStack[depth - 1];
for (var p in maybeCache) {
destinationCache[p] = maybeCache[p];
}
}
else {
// A false result goes straight into global cache (when something is false under assumptions it
// will also be false without assumptions)
relation[id] = false;
}
return result;
}
Expand Down Expand Up @@ -5399,7 +5412,7 @@ module ts {
return typeArgumentsAreAssignable;
}

function checkApplicableSignature(node: CallLikeExpression, args: Node[], signature: Signature, relation: Map<Ternary>, excludeArgument: boolean[], reportErrors: boolean) {
function checkApplicableSignature(node: CallLikeExpression, args: Node[], signature: Signature, relation: Map<boolean>, excludeArgument: boolean[], reportErrors: boolean) {
for (var i = 0; i < args.length; i++) {
var arg = args[i];
var argType: Type;
Expand Down Expand Up @@ -5593,7 +5606,7 @@ module ts {

return resolveErrorCall(node);

function chooseOverload(candidates: Signature[], relation: Map<Ternary>) {
function chooseOverload(candidates: Signature[], relation: Map<boolean>) {
for (var i = 0; i < candidates.length; i++) {
if (!hasCorrectArity(node, args, candidates[i])) {
continue;
Expand Down
21 changes: 21 additions & 0 deletions tests/baselines/reference/recursiveTypeComparison.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//// [recursiveTypeComparison.ts]
// Before fix this would take an exceeding long time to complete (#1170)

interface Observable<T> {
// This member can't be of type T, Property<T>, or Observable<anything but T>
needThisOne: Observable<T>;
// Add more to make it slower
expo1: Property<T[]>; // 0.31 seconds in check
expo2: Property<T[]>; // 3.11 seconds
expo3: Property<T[]>; // 82.28 seconds
}
interface Property<T> extends Observable<T> { }

var p: Observable<{}>;
var stuck: Property<number> = p;


//// [recursiveTypeComparison.js]
// Before fix this would take an exceeding long time to complete (#1170)
var p;
var stuck = p;
44 changes: 44 additions & 0 deletions tests/baselines/reference/recursiveTypeComparison.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
=== tests/cases/compiler/recursiveTypeComparison.ts ===
// Before fix this would take an exceeding long time to complete (#1170)

interface Observable<T> {
>Observable : Observable<T>
>T : T

// This member can't be of type T, Property<T>, or Observable<anything but T>
needThisOne: Observable<T>;
>needThisOne : Observable<T>
>Observable : Observable<T>
>T : T

// Add more to make it slower
expo1: Property<T[]>; // 0.31 seconds in check
>expo1 : Property<T[]>
>Property : Property<T>
>T : T

expo2: Property<T[]>; // 3.11 seconds
>expo2 : Property<T[]>
>Property : Property<T>
>T : T

expo3: Property<T[]>; // 82.28 seconds
>expo3 : Property<T[]>
>Property : Property<T>
>T : T
}
interface Property<T> extends Observable<T> { }
>Property : Property<T>
>T : T
>Observable : Observable<T>
>T : T

var p: Observable<{}>;
>p : Observable<{}>
>Observable : Observable<T>

var stuck: Property<number> = p;
>stuck : Property<number>
>Property : Property<T>
>p : Observable<{}>

36 changes: 36 additions & 0 deletions tests/baselines/reference/recursiveTypeComparison2.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
tests/cases/compiler/recursiveTypeComparison2.ts(13,80): error TS2304: Cannot find name 'StateValue'.


==== tests/cases/compiler/recursiveTypeComparison2.ts (1 errors) ====
// Before fix this would cause compiler to hang (#1170)

declare module Bacon {
interface Event<T> {
}
interface Error<T> extends Event<T> {
}
interface Observable<T> {
zip<U, V>(other: EventStream<U>, f: (a: T, b: U) => V): EventStream<V>;
slidingWindow(max: number, min?: number): Property<T[]>;
log(): Observable<T>;
combine<U, V>(other: Observable<U>, f: (a: T, b: U) => V): Property<V>;
withStateMachine<U, V>(initState: U, f: (state: U, event: Event<T>) => StateValue<U, V>): EventStream<V>;
~~~~~~~~~~~~~~~~
!!! error TS2304: Cannot find name 'StateValue'.
decode(mapping: Object): Property<any>;
awaiting<U>(other: Observable<U>): Property<boolean>;
endOnError(f?: (value: T) => boolean): Observable<T>;
withHandler(f: (event: Event<T>) => any): Observable<T>;
name(name: string): Observable<T>;
withDescription(...args: any[]): Observable<T>;
}
interface Property<T> extends Observable<T> {
}
interface EventStream<T> extends Observable<T> {
}
interface Bus<T> extends EventStream<T> {
}
var Bus: new <T>() => Bus<T>;
}

var stuck: Bacon.Bus<number> = new Bacon.Bus();
35 changes: 35 additions & 0 deletions tests/baselines/reference/recursiveTypeComparison2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//// [recursiveTypeComparison2.ts]
// Before fix this would cause compiler to hang (#1170)

declare module Bacon {
interface Event<T> {
}
interface Error<T> extends Event<T> {
}
interface Observable<T> {
zip<U, V>(other: EventStream<U>, f: (a: T, b: U) => V): EventStream<V>;
slidingWindow(max: number, min?: number): Property<T[]>;
log(): Observable<T>;
combine<U, V>(other: Observable<U>, f: (a: T, b: U) => V): Property<V>;
withStateMachine<U, V>(initState: U, f: (state: U, event: Event<T>) => StateValue<U, V>): EventStream<V>;
decode(mapping: Object): Property<any>;
awaiting<U>(other: Observable<U>): Property<boolean>;
endOnError(f?: (value: T) => boolean): Observable<T>;
withHandler(f: (event: Event<T>) => any): Observable<T>;
name(name: string): Observable<T>;
withDescription(...args: any[]): Observable<T>;
}
interface Property<T> extends Observable<T> {
}
interface EventStream<T> extends Observable<T> {
}
interface Bus<T> extends EventStream<T> {
}
var Bus: new <T>() => Bus<T>;
}

var stuck: Bacon.Bus<number> = new Bacon.Bus();

//// [recursiveTypeComparison2.js]
// Before fix this would cause compiler to hang (#1170)
var stuck = new Bacon.Bus();
14 changes: 14 additions & 0 deletions tests/cases/compiler/recursiveTypeComparison.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Before fix this would take an exceeding long time to complete (#1170)

interface Observable<T> {
// This member can't be of type T, Property<T>, or Observable<anything but T>
needThisOne: Observable<T>;
// Add more to make it slower
expo1: Property<T[]>; // 0.31 seconds in check
expo2: Property<T[]>; // 3.11 seconds
expo3: Property<T[]>; // 82.28 seconds
}
interface Property<T> extends Observable<T> { }

var p: Observable<{}>;
var stuck: Property<number> = p;
30 changes: 30 additions & 0 deletions tests/cases/compiler/recursiveTypeComparison2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Before fix this would cause compiler to hang (#1170)

declare module Bacon {
interface Event<T> {
}
interface Error<T> extends Event<T> {
}
interface Observable<T> {
zip<U, V>(other: EventStream<U>, f: (a: T, b: U) => V): EventStream<V>;
slidingWindow(max: number, min?: number): Property<T[]>;
log(): Observable<T>;
combine<U, V>(other: Observable<U>, f: (a: T, b: U) => V): Property<V>;
withStateMachine<U, V>(initState: U, f: (state: U, event: Event<T>) => StateValue<U, V>): EventStream<V>;
decode(mapping: Object): Property<any>;
awaiting<U>(other: Observable<U>): Property<boolean>;
endOnError(f?: (value: T) => boolean): Observable<T>;
withHandler(f: (event: Event<T>) => any): Observable<T>;
name(name: string): Observable<T>;
withDescription(...args: any[]): Observable<T>;
}
interface Property<T> extends Observable<T> {
}
interface EventStream<T> extends Observable<T> {
}
interface Bus<T> extends EventStream<T> {
}
var Bus: new <T>() => Bus<T>;
}

var stuck: Bacon.Bus<number> = new Bacon.Bus();