Skip to content
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

Wrap native map to work around 2**24 element limit #50310

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
102 changes: 81 additions & 21 deletions src/compiler/checker.ts
Expand Up @@ -347,6 +347,7 @@ namespace ts {
let totalInstantiationCount = 0;
let instantiationCount = 0;
let instantiationDepth = 0;
let nestedElementCacheContribution = 0;
let inlineLevel = 0;
let currentNode: Node | undefined;
let varianceTypeParameter: TypeParameter | undefined;
Expand Down Expand Up @@ -1042,12 +1043,52 @@ namespace ts {
let _jsxNamespace: __String;
let _jsxFactoryEntity: EntityName | undefined;

const subtypeRelation = new Map<string, RelationComparisonResult>();
const strictSubtypeRelation = new Map<string, RelationComparisonResult>();
const assignableRelation = new Map<string, RelationComparisonResult>();
const comparableRelation = new Map<string, RelationComparisonResult>();
const identityRelation = new Map<string, RelationComparisonResult>();
const enumRelation = new Map<string, RelationComparisonResult>();
class ExpandableRelationshipCache<K, V> {
private next?: ExpandableRelationshipCache<K, V>;
private inner: ESMap<K, V>;
constructor() {
this.inner = new Map();
}
get(key: K): V | undefined {
return this.inner.has(key) ? this.inner.get(key) : this.next?.get(key);
}
set(key: K, value: V): this {
if (this.inner.size > ((2 ** 24) - 1) && !this.inner.has(key)) {
this.next ||= new ExpandableRelationshipCache();
weswigham marked this conversation as resolved.
Show resolved Hide resolved
this.next.set(key, value);
}
else {
this.inner.set(key, value);
}
return this;
}
has(key: K): boolean {
return this.inner.has(key) || !!this.next?.has(key);
}
clear(): void {
this.inner.clear();
this.next?.clear();
this.next = undefined;
}
delete(key: K): boolean {
return this.inner.delete(key) || !!this.next?.delete(key);
}
forEach(callbackfn: (value: V, key: K, map: ExpandableRelationshipCache<K, V>) => void): void {
this.inner.forEach((v, k) => callbackfn(v, k, this));
this.next?.forEach((v, k) => callbackfn(v, k, this));
}
get size(): number {
return this.inner.size + (this.next?.size || 0);
}
}

type RelationCache = ExpandableRelationshipCache<string, RelationComparisonResult>;
const subtypeRelation = new ExpandableRelationshipCache<string, RelationComparisonResult>();
const strictSubtypeRelation = new ExpandableRelationshipCache<string, RelationComparisonResult>();
const assignableRelation = new ExpandableRelationshipCache<string, RelationComparisonResult>();
const comparableRelation = new ExpandableRelationshipCache<string, RelationComparisonResult>();
const identityRelation = new ExpandableRelationshipCache<string, RelationComparisonResult>();
const enumRelation = new ExpandableRelationshipCache<string, RelationComparisonResult>();

const builtinGlobals = createSymbolTable();
builtinGlobals.set(undefinedSymbol.escapedName, undefinedSymbol);
Expand Down Expand Up @@ -17583,7 +17624,7 @@ namespace ts {
function checkTypeRelatedToAndOptionallyElaborate(
source: Type,
target: Type,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
errorNode: Node | undefined,
expr: Expression | undefined,
headMessage: DiagnosticMessage | undefined,
Expand All @@ -17605,7 +17646,7 @@ namespace ts {
node: Expression | undefined,
source: Type,
target: Type,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
headMessage: DiagnosticMessage | undefined,
containingMessageChain: (() => DiagnosticMessageChain | undefined) | undefined,
errorOutputContainer: { errors?: Diagnostic[], skipLogging?: boolean } | undefined
Expand Down Expand Up @@ -17642,7 +17683,7 @@ namespace ts {
node: Expression,
source: Type,
target: Type,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
headMessage: DiagnosticMessage | undefined,
containingMessageChain: (() => DiagnosticMessageChain | undefined) | undefined,
errorOutputContainer: { errors?: Diagnostic[], skipLogging?: boolean } | undefined
Expand Down Expand Up @@ -17671,7 +17712,7 @@ namespace ts {
node: ArrowFunction,
source: Type,
target: Type,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
containingMessageChain: (() => DiagnosticMessageChain | undefined) | undefined,
errorOutputContainer: { errors?: Diagnostic[], skipLogging?: boolean } | undefined
): boolean {
Expand Down Expand Up @@ -17758,7 +17799,7 @@ namespace ts {
iterator: ElaborationIterator,
source: Type,
target: Type,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
containingMessageChain: (() => DiagnosticMessageChain | undefined) | undefined,
errorOutputContainer: { errors?: Diagnostic[], skipLogging?: boolean } | undefined
) {
Expand Down Expand Up @@ -17876,7 +17917,7 @@ namespace ts {
node: JsxAttributes,
source: Type,
target: Type,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
containingMessageChain: (() => DiagnosticMessageChain | undefined) | undefined,
errorOutputContainer: { errors?: Diagnostic[], skipLogging?: boolean } | undefined
) {
Expand Down Expand Up @@ -17977,7 +18018,7 @@ namespace ts {
node: ArrayLiteralExpression,
source: Type,
target: Type,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
containingMessageChain: (() => DiagnosticMessageChain | undefined) | undefined,
errorOutputContainer: { errors?: Diagnostic[], skipLogging?: boolean } | undefined
) {
Expand Down Expand Up @@ -18030,7 +18071,7 @@ namespace ts {
node: ObjectLiteralExpression,
source: Type,
target: Type,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
containingMessageChain: (() => DiagnosticMessageChain | undefined) | undefined,
errorOutputContainer: { errors?: Diagnostic[], skipLogging?: boolean } | undefined
) {
Expand Down Expand Up @@ -18334,7 +18375,7 @@ namespace ts {
return true;
}

function isSimpleTypeRelatedTo(source: Type, target: Type, relation: ESMap<string, RelationComparisonResult>, errorReporter?: ErrorReporter) {
function isSimpleTypeRelatedTo(source: Type, target: Type, relation: RelationCache, errorReporter?: ErrorReporter) {
const s = source.flags;
const t = target.flags;
if (t & TypeFlags.AnyOrUnknown || s & TypeFlags.Never || source === wildcardType) return true;
Expand Down Expand Up @@ -18375,7 +18416,7 @@ namespace ts {
return false;
}

function isTypeRelatedTo(source: Type, target: Type, relation: ESMap<string, RelationComparisonResult>) {
function isTypeRelatedTo(source: Type, target: Type, relation: RelationCache) {
if (isFreshLiteralType(source)) {
source = (source as FreshableType).regularType;
}
Expand Down Expand Up @@ -18452,7 +18493,7 @@ namespace ts {
function checkTypeRelatedTo(
source: Type,
target: Type,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
errorNode: Node | undefined,
headMessage?: DiagnosticMessage,
containingMessageChain?: () => DiagnosticMessageChain | undefined,
Expand Down Expand Up @@ -20991,7 +21032,7 @@ namespace ts {
* To improve caching, the relation key for two generic types uses the target's id plus ids of the type parameters.
* For other cases, the types ids are used.
*/
function getRelationKey(source: Type, target: Type, intersectionState: IntersectionState, relation: ESMap<string, RelationComparisonResult>, ignoreConstraints: boolean) {
function getRelationKey(source: Type, target: Type, intersectionState: IntersectionState, relation: RelationCache, ignoreConstraints: boolean) {
if (relation === identityRelation && source.id > target.id) {
const temp = source;
source = target;
Expand Down Expand Up @@ -30384,7 +30425,7 @@ namespace ts {
function checkApplicableSignatureForJsxOpeningLikeElement(
node: JsxOpeningLikeElement,
signature: Signature,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
checkMode: CheckMode,
reportErrors: boolean,
containingMessageChain: (() => DiagnosticMessageChain | undefined) | undefined,
Expand Down Expand Up @@ -30487,7 +30528,7 @@ namespace ts {
node: CallLikeExpression,
args: readonly Expression[],
signature: Signature,
relation: ESMap<string, RelationComparisonResult>,
relation: RelationCache,
checkMode: CheckMode,
reportErrors: boolean,
containingMessageChain: (() => DiagnosticMessageChain | undefined) | undefined,
Expand Down Expand Up @@ -31051,7 +31092,7 @@ namespace ts {
candidateForTypeArgumentError = oldCandidateForTypeArgumentError;
}

function chooseOverload(candidates: Signature[], relation: ESMap<string, RelationComparisonResult>, isSingleNonGenericCandidate: boolean, signatureHelpTrailingComma = false) {
function chooseOverload(candidates: Signature[], relation: RelationCache, isSingleNonGenericCandidate: boolean, signatureHelpTrailingComma = false) {
candidatesForArgumentError = undefined;
candidateForArgumentArityError = undefined;
candidateForTypeArgumentError = undefined;
Expand Down Expand Up @@ -41683,8 +41724,27 @@ namespace ts {
const saveCurrentNode = currentNode;
currentNode = node;
instantiationCount = 0;
const saveNestedElementCacheContribution = nestedElementCacheContribution;
nestedElementCacheContribution = 0;
const startCacheSize = assignableRelation.size;
checkSourceElementWorker(node);
const rawAssignabilityCacheContribution = assignableRelation.size - startCacheSize;
const assignabilityCacheContribution = rawAssignabilityCacheContribution - nestedElementCacheContribution;
nestedElementCacheContribution = saveNestedElementCacheContribution + assignabilityCacheContribution;
currentNode = saveCurrentNode;

// If a single source element triggers pulling in 1 million comparions, editor perf is likely very bad.
// Surprisingly, the choice of 1 million is not arbitrary - it's just under 2^20, the number of comparisons
// required to compare two 2^10-element unions naively in the worst case. That is juuust large enough to take
// a few seconds to check on a laptop, and thus for things to not *obviously* be wrong without an error.
// Two 2^12 unions takes about a minute. Larger powers of two aren't worth waiting for, and cause an obvious hang.
// The error is sometimes safe to `//@ts-ignore` if the perf is still OK, but does indicate a location
// that is likely triggering a performance problem. The types may be able to be restructured to
// get better perf, or the constructs in use may be a good candidate for specialized optimizations
// in the compiler itself to reduce the amount of work required.
if (assignabilityCacheContribution > 1_000_000) {
error(node, Diagnostics.This_source_element_is_ultimately_responsible_for_0_million_type_comparisons_It_is_likely_very_slow_and_may_impact_editor_performance_Simplify_the_types_in_use, Math.floor(assignabilityCacheContribution / 1_000_000));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside, since the error is about cache sizes, it's inherently unstable - the first location to do these comparisons will get this error, others that do the same comparisons will not. Historically, location-unstable errors like this haven't been good for incremental build and the like, since the errors shifting mucks with up-to-dateness checks. This could maybe be worked around by indicating this error status in the relationship cache itself (eg, this top-level comparison triggered this overflow, so report the complexity error at every site where this comparison is performed), but given the likely rarity of this error, I'm not sure how worth it that'd be to do, given the complexity that'd entail compared to how simple this is.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weswigham I like the detailed explanation, thank you 👍 . I think, acknowledging the instability in cache sizes and its potential impact on incremental builds is crucial. Regarding the error status, I wonder if there's a simpler way to provide consistent feedback for similar comparisons without compromising complexity. Perhaps exploring a middle ground could help maintain simplicity while addressing occasional instability?

}
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Expand Up @@ -6239,6 +6239,10 @@
"category": "Error",
"code": 7061
},
"This source element is ultimately responsible for {0} million type comparisons. It is likely very slow, and may impact editor performance. Simplify the types in use.": {
"category": "Error",
"code": 7062
},

"You cannot rename this element.": {
"category": "Error",
Expand Down
34 changes: 34 additions & 0 deletions tests/baselines/reference/comparisonCountLimits.errors.txt
@@ -0,0 +1,34 @@
tests/cases/compiler/comparisonCountLimits.ts(9,17): error TS2590: Expression produces a union type that is too complex to represent.
tests/cases/compiler/comparisonCountLimits.ts(25,1): error TS7062: This source element is ultimately responsible for 1 million type comparisons. It is likely very slow, and may impact editor performance. Simplify the types in use.


==== tests/cases/compiler/comparisonCountLimits.ts (2 errors) ====
function get10BitsOf<T extends string, U extends string, FallbackPrefix extends string>() {
type Bit = T | U; // 2^1
type HalfNibble = `${Bit}${Bit}`; // 2^2
type Nibble = `${HalfNibble}${HalfNibble}`; // 2^4
type Byte = `${Nibble}${Nibble}`; // 2^8
type TenBits = `${Byte}${HalfNibble}`; // 2^10 (approx. 1 million comparisons if compared naively)

type HalfWord = `${Byte}${Byte}`; // 2^16 // allowed, but test takes way too long if used
type Word = `${HalfWord}${HalfWord}`; // 2^32 (throws, too large)
~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2590: Expression produces a union type that is too complex to represent.

// Literal type relations are uncached (lol), so everything has to be wrapped in an object to affect cache sizes
// (A distributive conditional is the easiest way to do the mapping, but others are possible, eg, mapped types,
// or explicit construction)
type Box<T> = T extends unknown ? {item: T} : never;

// By manufacturing the fallback in here, we guarantee it has a higher typeid than the bit strings,
// and thus is sorted to the end of the union, guaranteeing relationship checking passes with a maximal
// number of comparisons when a naive comparison is done (guaranteeing this test is slow)
return null as any as Box<TenBits | (FallbackPrefix extends never ? never : `${FallbackPrefix}${string}`)>; // return type is a union
}

let a = get10BitsOf<"0", "1", "a" | "b">();
const b = get10BitsOf<"a", "b", never>();

a = b;
~~~~~~
!!! error TS7062: This source element is ultimately responsible for 1 million type comparisons. It is likely very slow, and may impact editor performance. Simplify the types in use.
37 changes: 37 additions & 0 deletions tests/baselines/reference/comparisonCountLimits.js
@@ -0,0 +1,37 @@
//// [comparisonCountLimits.ts]
function get10BitsOf<T extends string, U extends string, FallbackPrefix extends string>() {
type Bit = T | U; // 2^1
type HalfNibble = `${Bit}${Bit}`; // 2^2
type Nibble = `${HalfNibble}${HalfNibble}`; // 2^4
type Byte = `${Nibble}${Nibble}`; // 2^8
type TenBits = `${Byte}${HalfNibble}`; // 2^10 (approx. 1 million comparisons if compared naively)

type HalfWord = `${Byte}${Byte}`; // 2^16 // allowed, but test takes way too long if used
type Word = `${HalfWord}${HalfWord}`; // 2^32 (throws, too large)

// Literal type relations are uncached (lol), so everything has to be wrapped in an object to affect cache sizes
// (A distributive conditional is the easiest way to do the mapping, but others are possible, eg, mapped types,
// or explicit construction)
type Box<T> = T extends unknown ? {item: T} : never;

// By manufacturing the fallback in here, we guarantee it has a higher typeid than the bit strings,
// and thus is sorted to the end of the union, guaranteeing relationship checking passes with a maximal
// number of comparisons when a naive comparison is done (guaranteeing this test is slow)
return null as any as Box<TenBits | (FallbackPrefix extends never ? never : `${FallbackPrefix}${string}`)>; // return type is a union
}

let a = get10BitsOf<"0", "1", "a" | "b">();
const b = get10BitsOf<"a", "b", never>();

a = b;

//// [comparisonCountLimits.js]
function get10BitsOf() {
// By manufacturing the fallback in here, we guarantee it has a higher typeid than the bit strings,
// and thus is sorted to the end of the union, guaranteeing relationship checking passes with a maximal
// number of comparisons when a naive comparison is done (guaranteeing this test is slow)
return null; // return type is a union
}
var a = get10BitsOf();
var b = get10BitsOf();
a = b;
27 changes: 27 additions & 0 deletions tests/cases/compiler/comparisonCountLimits.ts
@@ -0,0 +1,27 @@
// @noTypesAndSymbols: true

function get10BitsOf<T extends string, U extends string, FallbackPrefix extends string>() {
type Bit = T | U; // 2^1
type HalfNibble = `${Bit}${Bit}`; // 2^2
type Nibble = `${HalfNibble}${HalfNibble}`; // 2^4
type Byte = `${Nibble}${Nibble}`; // 2^8
type TenBits = `${Byte}${HalfNibble}`; // 2^10 (approx. 1 million comparisons if compared naively)

type HalfWord = `${Byte}${Byte}`; // 2^16 // allowed, but test takes way too long if used
type Word = `${HalfWord}${HalfWord}`; // 2^32 (throws, too large)

// Literal type relations are uncached (lol), so everything has to be wrapped in an object to affect cache sizes
// (A distributive conditional is the easiest way to do the mapping, but others are possible, eg, mapped types,
// or explicit construction)
type Box<T> = T extends unknown ? {item: T} : never;

// By manufacturing the fallback in here, we guarantee it has a higher typeid than the bit strings,
// and thus is sorted to the end of the union, guaranteeing relationship checking passes with a maximal
// number of comparisons when a naive comparison is done (guaranteeing this test is slow)
return null as any as Box<TenBits | (FallbackPrefix extends never ? never : `${FallbackPrefix}${string}`)>; // return type is a union
}

let a = get10BitsOf<"0", "1", "a" | "b">();
const b = get10BitsOf<"a", "b", never>();

a = b;