From 9de8ff3439f44f279b41346f2ad4894e43251e9b Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 27 Jun 2024 00:52:58 +0000 Subject: [PATCH 01/13] Use a `Set` and a binary search to find longest-prefix matches more quickly. This also adds a naive cache for each call, though it is not resilient to being passed the same array of patterns twice with different contents, and could be defeated if the patterns frequently differ. --- src/compiler/core.ts | 4 +- src/compiler/utilities.ts | 88 +++++++++++++++++++++++++++++++++++---- 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 977da801ed946..f6fc7e6f9bcab 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -2532,12 +2532,12 @@ export function matchedText(pattern: Pattern, candidate: string): string { * * @internal */ -export function findBestPatternMatch(values: readonly T[], getPattern: (value: T) => Pattern, candidate: string): T | undefined { +export function findBestPatternMatch(values: readonly T[], getPattern: (value: T) => Pattern, candidate: string, endIndex: number = values.length): T | undefined { let matchedValue: T | undefined; // use length of prefix as betterness criteria let longestMatchPrefixLength = -1; - for (let i = 0; i < values.length; i++) { + for (let i = 0; i < endIndex; i++) { const v = values[i]; const pattern = getPattern(v); if (isPatternMatch(pattern, candidate) && pattern.prefix.length > longestMatchPrefixLength) { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 99cb0c7f869da..cec8ffe605312 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -28,6 +28,7 @@ import { BarBarEqualsToken, BinaryExpression, binarySearch, + binarySearchKey, BindableObjectDefinePropertyCall, BindableStaticAccessExpression, BindableStaticElementAccessExpression, @@ -339,6 +340,7 @@ import { isParameterPropertyDeclaration, isParenthesizedExpression, isParenthesizedTypeNode, + isPatternMatch, isPrefixUnaryExpression, isPrivateIdentifier, isPropertyAccessExpression, @@ -10061,6 +10063,10 @@ export const emptyFileSystemEntries: FileSystemEntries = { directories: emptyArray, }; +let lastPatternOrStringsArray: readonly (string | Pattern)[]; +let matchableStringSet: Set; +let sortedPatterns: Pattern[]; + /** * patternOrStrings contains both patterns (containing "*") and regular strings. * Return an exact match if possible, or a pattern match, or undefined. @@ -10069,18 +10075,86 @@ export const emptyFileSystemEntries: FileSystemEntries = { * @internal */ export function matchPatternOrExact(patternOrStrings: readonly (string | Pattern)[], candidate: string): string | Pattern | undefined { - const patterns: Pattern[] = []; - for (const patternOrString of patternOrStrings) { - if (patternOrString === candidate) { - return candidate; + if (patternOrStrings !== lastPatternOrStringsArray) { + lastPatternOrStringsArray = patternOrStrings; + matchableStringSet = new Set(); + sortedPatterns = []; + + // TODO: avoid falling out of the below logic. + + for (const patternOrString of patternOrStrings) { + if (typeof patternOrString === "string") { + matchableStringSet.add(patternOrString); + } + else { + sortedPatterns.push(patternOrString); + } + } + + sortedPatterns.sort((a, b) => compareStringsCaseSensitive(a.prefix, b.prefix)); + } + + if (matchableStringSet.has(candidate)) { + return candidate; + } + if (sortedPatterns.length === 0) { + return undefined; + } + + let index = binarySearchKey(sortedPatterns, candidate, getPatternPrefix, compareStringsCaseSensitive); + if (index < 0) { + index = ~index; + } + + if (index >= sortedPatterns.length) { + // If we are past the end of the array, then the candidate length just exceeds the last prefix. + // Bump us back into a reasonable range. + index--; + } + + // `sortedPatterns` is sorted by prefixes, where longer prefixes should occur later; + // however, the sort is stable, so the original input order of patterns is preserved within each group. + // So for something like + // + // { prefix: "foo/", suffix: "bar"}, { "prefix: "", suffix: "" }, { prefix: "foo/", suffix: "" }, { "prefix: "", suffix: "bar" }, + // + // we will end up with + // + // { "prefix: "", suffix: "" }, { "prefix: "", suffix: "bar" }, { prefix: "foo/", suffix: "bar"}, { prefix: "foo/", suffix: "" } + // + // guaranteeing that within a group, the first match is ideal. + // + // Now the binary search may have landed us in the very middle of a group. If we are searching for "foo/" in + // + // ..., { prefix: "foo/", suffix: "" }, { prefix: "foo/", suffix: "my-suffix" }, ... + // + // then we could have ended up on an exact match. Keep walking backwards on exact matches until we find the first. + // This will allow us to try out all patterns with an identical prefix, while maintaining the relative original order of the + // patterns as specified. + const groupPrefix = sortedPatterns[index].prefix; + while (index > 0 && groupPrefix === sortedPatterns[index - 1].prefix) { + index--; + } + + for (let i = index; i < sortedPatterns.length; i++) { + const currentPattern = sortedPatterns[i]; + if (currentPattern.prefix !== groupPrefix) { + break; } - if (!isString(patternOrString)) { - patterns.push(patternOrString); + if (isPatternMatch(currentPattern, candidate)) { + return currentPattern; } } - return findBestPatternMatch(patterns, _ => _, candidate); + // We could not find a pattern within the group that matched. + // Technically we could walk backwards from here and find likely matches. + // For now, we'll just do a simple linear search up to the current group index. + return findBestPatternMatch(sortedPatterns, _ => _, candidate, /*endIndex*/ index); +} + +function getPatternPrefix(pattern: Pattern) { + return pattern.prefix; } /** @internal */ From 347908caa16a7be595a185fa49e972364ce3a299 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 27 Jun 2024 18:22:46 +0000 Subject: [PATCH 02/13] Cache more than one input array at a time to avoid redoing work in a ping/pong situation. --- src/compiler/utilities.ts | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index cec8ffe605312..393b9c5ddd78a 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10063,9 +10063,12 @@ export const emptyFileSystemEntries: FileSystemEntries = { directories: emptyArray, }; -let lastPatternOrStringsArray: readonly (string | Pattern)[]; -let matchableStringSet: Set; -let sortedPatterns: Pattern[]; +interface MatchPatternOrExactCacheEntry { + matchableStringSet: Set; + sortedPatterns: Pattern[]; +} + +const patternOrStringsCache = new WeakMap() /** * patternOrStrings contains both patterns (containing "*") and regular strings. @@ -10075,13 +10078,17 @@ let sortedPatterns: Pattern[]; * @internal */ export function matchPatternOrExact(patternOrStrings: readonly (string | Pattern)[], candidate: string): string | Pattern | undefined { - if (patternOrStrings !== lastPatternOrStringsArray) { - lastPatternOrStringsArray = patternOrStrings; + let matchableStringSet: Set; + let sortedPatterns: Pattern[]; + + const cacheEntry = patternOrStringsCache.get(patternOrStrings); + if (cacheEntry !== undefined) { + ({ matchableStringSet, sortedPatterns } = cacheEntry); + } + else { matchableStringSet = new Set(); sortedPatterns = []; - // TODO: avoid falling out of the below logic. - for (const patternOrString of patternOrStrings) { if (typeof patternOrString === "string") { matchableStringSet.add(patternOrString); @@ -10092,6 +10099,11 @@ export function matchPatternOrExact(patternOrStrings: readonly (string | Pattern } sortedPatterns.sort((a, b) => compareStringsCaseSensitive(a.prefix, b.prefix)); + + patternOrStringsCache.set(patternOrStrings, { + matchableStringSet, + sortedPatterns, + }) } if (matchableStringSet.has(candidate)) { From 5b0c17e13980b472d9d0e0bb5b7d0dea97af239a Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 27 Jun 2024 18:27:02 +0000 Subject: [PATCH 03/13] Format --- src/compiler/utilities.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 393b9c5ddd78a..2af5fc126dbc9 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10068,7 +10068,7 @@ interface MatchPatternOrExactCacheEntry { sortedPatterns: Pattern[]; } -const patternOrStringsCache = new WeakMap() +const patternOrStringsCache = new WeakMap(); /** * patternOrStrings contains both patterns (containing "*") and regular strings. @@ -10103,7 +10103,7 @@ export function matchPatternOrExact(patternOrStrings: readonly (string | Pattern patternOrStringsCache.set(patternOrStrings, { matchableStringSet, sortedPatterns, - }) + }); } if (matchableStringSet.has(candidate)) { From 4a88ba638aa1b470caae9b0551125c0e519a2a79 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 27 Jun 2024 19:20:19 +0000 Subject: [PATCH 04/13] Cache calculated `pathPatterns` to avoid recreating them. --- src/compiler/moduleNameResolver.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 59a352a223547..221f2412c970a 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -1568,7 +1568,7 @@ function tryLoadModuleUsingPathsIfEligible(extensions: Extensions, moduleName: s trace(state.host, Diagnostics.paths_option_is_specified_looking_for_a_pattern_to_match_module_name_0, moduleName); } const baseDirectory = getPathsBasePath(state.compilerOptions, state.host)!; // Always defined when 'paths' is defined - const pathPatterns = configFile?.configFileSpecs ? configFile.configFileSpecs.pathPatterns ||= tryParsePatterns(paths) : undefined; + const pathPatterns = configFile?.configFileSpecs ? configFile.configFileSpecs.pathPatterns ??= tryParsePatterns(paths) : undefined; return tryLoadModuleUsingPaths(extensions, moduleName, baseDirectory, paths, pathPatterns, loader, /*onlyRecordFailures*/ false, state); } } @@ -3126,8 +3126,14 @@ function loadModuleFromSpecificNodeModulesDirectory(extensions: Extensions, modu return loader(extensions, candidate, !nodeModulesDirectoryExists, state); } +const pathsToPatternsCache = new WeakMap, readonly (string | Pattern)[]>(); + function tryLoadModuleUsingPaths(extensions: Extensions, moduleName: string, baseDirectory: string, paths: MapLike, pathPatterns: readonly (string | Pattern)[] | undefined, loader: ResolutionKindSpecificLoader, onlyRecordFailures: boolean, state: ModuleResolutionState): SearchResult { - pathPatterns ||= tryParsePatterns(paths); + pathPatterns ??= pathsToPatternsCache.get(paths); + if (pathPatterns === undefined) { + pathsToPatternsCache.set(paths, pathPatterns = tryParsePatterns(paths)); + } + const matchedPattern = matchPatternOrExact(pathPatterns, moduleName); if (matchedPattern) { const matchedStar = isString(matchedPattern) ? undefined : matchedText(matchedPattern, moduleName); From 123b7c3ff0226a7fee39b3585db1abcafd77ce21 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 28 Jun 2024 00:26:11 +0000 Subject: [PATCH 05/13] Move caching to `tryParsePatterns`, avoid creating an intermediate array entirely. --- src/compiler/commandLineParser.ts | 1 - src/compiler/moduleNameResolver.ts | 24 ++++---- src/compiler/types.ts | 1 - src/compiler/utilities.ts | 95 ++++++++++++++++++------------ 4 files changed, 67 insertions(+), 54 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 5c0a33c6e46bb..4368d1039ce20 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -3042,7 +3042,6 @@ function parseJsonConfigFileContentWorker( validatedFilesSpecBeforeSubstitution, validatedIncludeSpecsBeforeSubstitution, validatedExcludeSpecsBeforeSubstitution, - pathPatterns: undefined, // Initialized on first use isDefaultIncludeSpec, }; } diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 221f2412c970a..5291540dee4e4 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -79,9 +79,9 @@ import { normalizeSlashes, PackageId, packageIdToString, + ParsedPatterns, Path, pathIsRelative, - Pattern, patternText, readJson, removeExtension, @@ -1559,7 +1559,7 @@ function tryLoadModuleUsingOptionalResolutionSettings(extensions: Extensions, mo } function tryLoadModuleUsingPathsIfEligible(extensions: Extensions, moduleName: string, loader: ResolutionKindSpecificLoader, state: ModuleResolutionState) { - const { baseUrl, paths, configFile } = state.compilerOptions; + const { baseUrl, paths } = state.compilerOptions; if (paths && !pathIsRelative(moduleName)) { if (state.traceEnabled) { if (baseUrl) { @@ -1568,7 +1568,8 @@ function tryLoadModuleUsingPathsIfEligible(extensions: Extensions, moduleName: s trace(state.host, Diagnostics.paths_option_is_specified_looking_for_a_pattern_to_match_module_name_0, moduleName); } const baseDirectory = getPathsBasePath(state.compilerOptions, state.host)!; // Always defined when 'paths' is defined - const pathPatterns = configFile?.configFileSpecs ? configFile.configFileSpecs.pathPatterns ??= tryParsePatterns(paths) : undefined; + // TODO: do we need to sort by aggregate length? + const pathPatterns = tryParsePatterns(paths); return tryLoadModuleUsingPaths(extensions, moduleName, baseDirectory, paths, pathPatterns, loader, /*onlyRecordFailures*/ false, state); } } @@ -2524,7 +2525,9 @@ function loadNodeModuleFromDirectoryWorker(extensions: Extensions, candidate: st if (state.traceEnabled) { trace(state.host, Diagnostics.package_json_has_a_typesVersions_entry_0_that_matches_compiler_version_1_looking_for_a_pattern_to_match_module_name_2, versionPaths.version, version, moduleName); } - const result = tryLoadModuleUsingPaths(extensions, moduleName, candidate, versionPaths.paths, /*pathPatterns*/ undefined, loader, onlyRecordFailuresForPackageFile || onlyRecordFailuresForIndex, state); + // TODO: do we need to sort by aggregate length? + const pathPatterns = tryParsePatterns(versionPaths.paths); + const result = tryLoadModuleUsingPaths(extensions, moduleName, candidate, versionPaths.paths, pathPatterns, loader, onlyRecordFailuresForPackageFile || onlyRecordFailuresForIndex, state); if (result) { return removeIgnoredPackageId(result.value); } @@ -3118,7 +3121,9 @@ function loadModuleFromSpecificNodeModulesDirectory(extensions: Extensions, modu trace(state.host, Diagnostics.package_json_has_a_typesVersions_entry_0_that_matches_compiler_version_1_looking_for_a_pattern_to_match_module_name_2, versionPaths.version, version, rest); } const packageDirectoryExists = nodeModulesDirectoryExists && directoryProbablyExists(packageDirectory, state.host); - const fromPaths = tryLoadModuleUsingPaths(extensions, rest, packageDirectory, versionPaths.paths, /*pathPatterns*/ undefined, loader, !packageDirectoryExists, state); + // TODO: do we need to sort by aggregate length? + const pathPatterns = tryParsePatterns(versionPaths.paths); + const fromPaths = tryLoadModuleUsingPaths(extensions, rest, packageDirectory, versionPaths.paths, pathPatterns, loader, !packageDirectoryExists, state); if (fromPaths) { return fromPaths.value; } @@ -3126,14 +3131,7 @@ function loadModuleFromSpecificNodeModulesDirectory(extensions: Extensions, modu return loader(extensions, candidate, !nodeModulesDirectoryExists, state); } -const pathsToPatternsCache = new WeakMap, readonly (string | Pattern)[]>(); - -function tryLoadModuleUsingPaths(extensions: Extensions, moduleName: string, baseDirectory: string, paths: MapLike, pathPatterns: readonly (string | Pattern)[] | undefined, loader: ResolutionKindSpecificLoader, onlyRecordFailures: boolean, state: ModuleResolutionState): SearchResult { - pathPatterns ??= pathsToPatternsCache.get(paths); - if (pathPatterns === undefined) { - pathsToPatternsCache.set(paths, pathPatterns = tryParsePatterns(paths)); - } - +function tryLoadModuleUsingPaths(extensions: Extensions, moduleName: string, baseDirectory: string, paths: MapLike, pathPatterns: ParsedPatterns, loader: ResolutionKindSpecificLoader, onlyRecordFailures: boolean, state: ModuleResolutionState): SearchResult { const matchedPattern = matchPatternOrExact(pathPatterns, moduleName); if (matchedPattern) { const matchedStar = isString(matchedPattern) ? undefined : matchedText(matchedPattern, moduleName); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 33f1b459215f6..53e0de202c9a4 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -7512,7 +7512,6 @@ export interface ConfigFileSpecs { validatedFilesSpecBeforeSubstitution: readonly string[] | undefined; validatedIncludeSpecsBeforeSubstitution: readonly string[] | undefined; validatedExcludeSpecsBeforeSubstitution: readonly string[] | undefined; - pathPatterns: readonly (string | Pattern)[] | undefined; isDefaultIncludeSpec: boolean; } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 2af5fc126dbc9..dce4129b87890 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10006,8 +10006,57 @@ export function tryParsePattern(pattern: string): string | Pattern | undefined { } /** @internal */ -export function tryParsePatterns(paths: MapLike): (string | Pattern)[] { - return mapDefined(getOwnKeys(paths), path => tryParsePattern(path)); +export interface ParsedPatterns { + matchableStringSet: ReadonlySet | undefined; + sortedPatterns: (readonly Pattern[]) | undefined; +} + +const parsedPatternsCache = new WeakMap, ParsedPatterns>(); + +/** + * Divides patterns into a set of exact specifiers and sorted patterns. + * NOTE that this function caches, and assumes the same `paths` argument will + * never be provided with a different value for `sortByAggregateLength`. + * + * @internal + **/ +export function tryParsePatterns(paths: MapLike, sortByAggregateLength: boolean = false): ParsedPatterns { + let result = parsedPatternsCache.get(paths) + if (result !== undefined) { + return result; + } + + let matchableStringSet: Set | undefined; + let sortedPatterns: Pattern[] | undefined; + + const pathList = getOwnKeys(paths); + for (const path of pathList) { + const patternOrStr = tryParsePattern(path) + if (patternOrStr === undefined) { + continue; + } + else if (typeof patternOrStr === "string") { + (matchableStringSet ??= new Set()).add(patternOrStr); + } + else { + (sortedPatterns ??= []).push(patternOrStr); + } + } + + sortedPatterns?.sort((a, b) => { + const prefixComparison = compareStringsCaseSensitive(a.prefix, b.prefix) + if (prefixComparison === 0 && sortByAggregateLength) { + return a.suffix.length - b.suffix.length; + } + return prefixComparison; + }); + + parsedPatternsCache.set(paths, result = { + matchableStringSet, + sortedPatterns, + }); + + return result; } /** @internal */ @@ -10063,13 +10112,6 @@ export const emptyFileSystemEntries: FileSystemEntries = { directories: emptyArray, }; -interface MatchPatternOrExactCacheEntry { - matchableStringSet: Set; - sortedPatterns: Pattern[]; -} - -const patternOrStringsCache = new WeakMap(); - /** * patternOrStrings contains both patterns (containing "*") and regular strings. * Return an exact match if possible, or a pattern match, or undefined. @@ -10077,39 +10119,14 @@ const patternOrStringsCache = new WeakMap; - let sortedPatterns: Pattern[]; - - const cacheEntry = patternOrStringsCache.get(patternOrStrings); - if (cacheEntry !== undefined) { - ({ matchableStringSet, sortedPatterns } = cacheEntry); - } - else { - matchableStringSet = new Set(); - sortedPatterns = []; - - for (const patternOrString of patternOrStrings) { - if (typeof patternOrString === "string") { - matchableStringSet.add(patternOrString); - } - else { - sortedPatterns.push(patternOrString); - } - } - - sortedPatterns.sort((a, b) => compareStringsCaseSensitive(a.prefix, b.prefix)); +export function matchPatternOrExact(patternOrStrings: ParsedPatterns, candidate: string): string | Pattern | undefined { + const { matchableStringSet, sortedPatterns } = patternOrStrings; - patternOrStringsCache.set(patternOrStrings, { - matchableStringSet, - sortedPatterns, - }); - } - - if (matchableStringSet.has(candidate)) { + if (matchableStringSet?.has(candidate)) { return candidate; } - if (sortedPatterns.length === 0) { + + if (sortedPatterns === undefined || sortedPatterns.length === 0) { return undefined; } From 3d4bc20c2af8ceb87dad1b2ac503d5ae84aa98a8 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 28 Jun 2024 00:36:16 +0000 Subject: [PATCH 06/13] Format. --- src/compiler/utilities.ts | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index dce4129b87890..560fc7b90ac28 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10017,21 +10017,21 @@ const parsedPatternsCache = new WeakMap, ParsedPatterns>(); * Divides patterns into a set of exact specifiers and sorted patterns. * NOTE that this function caches, and assumes the same `paths` argument will * never be provided with a different value for `sortByAggregateLength`. - * + * * @internal - **/ + */ export function tryParsePatterns(paths: MapLike, sortByAggregateLength: boolean = false): ParsedPatterns { - let result = parsedPatternsCache.get(paths) + let result = parsedPatternsCache.get(paths); if (result !== undefined) { return result; } let matchableStringSet: Set | undefined; let sortedPatterns: Pattern[] | undefined; - + const pathList = getOwnKeys(paths); for (const path of pathList) { - const patternOrStr = tryParsePattern(path) + const patternOrStr = tryParsePattern(path); if (patternOrStr === undefined) { continue; } @@ -10044,17 +10044,20 @@ export function tryParsePatterns(paths: MapLike, sortByAggregateLength } sortedPatterns?.sort((a, b) => { - const prefixComparison = compareStringsCaseSensitive(a.prefix, b.prefix) + const prefixComparison = compareStringsCaseSensitive(a.prefix, b.prefix); if (prefixComparison === 0 && sortByAggregateLength) { return a.suffix.length - b.suffix.length; } return prefixComparison; }); - parsedPatternsCache.set(paths, result = { - matchableStringSet, - sortedPatterns, - }); + parsedPatternsCache.set( + paths, + result = { + matchableStringSet, + sortedPatterns, + }, + ); return result; } From 8919029ce8d49c244dd9698d5cc2539bac63680a Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 28 Jun 2024 01:29:10 +0000 Subject: [PATCH 07/13] Avoid leaning on the WeakMap when we can cache on `ConfigFileSpecs`. --- src/compiler/commandLineParser.ts | 1 + src/compiler/moduleNameResolver.ts | 12 ++++++++++-- src/compiler/types.ts | 2 ++ src/compiler/utilities.ts | 26 +++++++++++++++----------- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 4368d1039ce20..5c0a33c6e46bb 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -3042,6 +3042,7 @@ function parseJsonConfigFileContentWorker( validatedFilesSpecBeforeSubstitution, validatedIncludeSpecsBeforeSubstitution, validatedExcludeSpecsBeforeSubstitution, + pathPatterns: undefined, // Initialized on first use isDefaultIncludeSpec, }; } diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 5291540dee4e4..64fc43b538f85 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -1559,7 +1559,7 @@ function tryLoadModuleUsingOptionalResolutionSettings(extensions: Extensions, mo } function tryLoadModuleUsingPathsIfEligible(extensions: Extensions, moduleName: string, loader: ResolutionKindSpecificLoader, state: ModuleResolutionState) { - const { baseUrl, paths } = state.compilerOptions; + const { baseUrl, paths, configFile } = state.compilerOptions; if (paths && !pathIsRelative(moduleName)) { if (state.traceEnabled) { if (baseUrl) { @@ -1569,7 +1569,15 @@ function tryLoadModuleUsingPathsIfEligible(extensions: Extensions, moduleName: s } const baseDirectory = getPathsBasePath(state.compilerOptions, state.host)!; // Always defined when 'paths' is defined // TODO: do we need to sort by aggregate length? - const pathPatterns = tryParsePatterns(paths); + let pathPatterns; + if (configFile?.configFileSpecs) { + // Cache on this object instead of internally through a WeakMap. + // This seems to do a bit better. + pathPatterns = configFile.configFileSpecs.pathPatterns ??= tryParsePatterns(paths, /*shouldCache*/ false); + } + else { + pathPatterns = tryParsePatterns(paths); + } return tryLoadModuleUsingPaths(extensions, moduleName, baseDirectory, paths, pathPatterns, loader, /*onlyRecordFailures*/ false, state); } } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 53e0de202c9a4..3ee2a197e1bbd 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -12,6 +12,7 @@ import { OptionsNameMap, PackageJsonInfo, PackageJsonInfoCache, + ParsedPatterns, Pattern, SymlinkCache, ThisContainer, @@ -7512,6 +7513,7 @@ export interface ConfigFileSpecs { validatedFilesSpecBeforeSubstitution: readonly string[] | undefined; validatedIncludeSpecsBeforeSubstitution: readonly string[] | undefined; validatedExcludeSpecsBeforeSubstitution: readonly string[] | undefined; + pathPatterns: ParsedPatterns | undefined; isDefaultIncludeSpec: boolean; } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 560fc7b90ac28..48785426c60bd 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10020,10 +10020,13 @@ const parsedPatternsCache = new WeakMap, ParsedPatterns>(); * * @internal */ -export function tryParsePatterns(paths: MapLike, sortByAggregateLength: boolean = false): ParsedPatterns { - let result = parsedPatternsCache.get(paths); - if (result !== undefined) { - return result; +export function tryParsePatterns(paths: MapLike, shouldCache: boolean = true, sortByAggregateLength: boolean = false): ParsedPatterns { + let result: ParsedPatterns | undefined; + if (shouldCache) { + result = parsedPatternsCache.get(paths); + if (result !== undefined) { + return result; + } } let matchableStringSet: Set | undefined; @@ -10051,13 +10054,14 @@ export function tryParsePatterns(paths: MapLike, sortByAggregateLength return prefixComparison; }); - parsedPatternsCache.set( - paths, - result = { - matchableStringSet, - sortedPatterns, - }, - ); + result = { + matchableStringSet, + sortedPatterns, + }; + + if (shouldCache) { + parsedPatternsCache.set(paths, result); + } return result; } From 75835bbe219ddff83b4d1ab8802ec31f4d433a46 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 28 Jun 2024 18:38:52 +0000 Subject: [PATCH 08/13] Revert "Avoid leaning on the WeakMap when we can cache on `ConfigFileSpecs`." This reverts commit 8919029ce8d49c244dd9698d5cc2539bac63680a. --- src/compiler/commandLineParser.ts | 1 - src/compiler/moduleNameResolver.ts | 12 ++---------- src/compiler/types.ts | 2 -- src/compiler/utilities.ts | 26 +++++++++++--------------- 4 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 5c0a33c6e46bb..4368d1039ce20 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -3042,7 +3042,6 @@ function parseJsonConfigFileContentWorker( validatedFilesSpecBeforeSubstitution, validatedIncludeSpecsBeforeSubstitution, validatedExcludeSpecsBeforeSubstitution, - pathPatterns: undefined, // Initialized on first use isDefaultIncludeSpec, }; } diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 64fc43b538f85..5291540dee4e4 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -1559,7 +1559,7 @@ function tryLoadModuleUsingOptionalResolutionSettings(extensions: Extensions, mo } function tryLoadModuleUsingPathsIfEligible(extensions: Extensions, moduleName: string, loader: ResolutionKindSpecificLoader, state: ModuleResolutionState) { - const { baseUrl, paths, configFile } = state.compilerOptions; + const { baseUrl, paths } = state.compilerOptions; if (paths && !pathIsRelative(moduleName)) { if (state.traceEnabled) { if (baseUrl) { @@ -1569,15 +1569,7 @@ function tryLoadModuleUsingPathsIfEligible(extensions: Extensions, moduleName: s } const baseDirectory = getPathsBasePath(state.compilerOptions, state.host)!; // Always defined when 'paths' is defined // TODO: do we need to sort by aggregate length? - let pathPatterns; - if (configFile?.configFileSpecs) { - // Cache on this object instead of internally through a WeakMap. - // This seems to do a bit better. - pathPatterns = configFile.configFileSpecs.pathPatterns ??= tryParsePatterns(paths, /*shouldCache*/ false); - } - else { - pathPatterns = tryParsePatterns(paths); - } + const pathPatterns = tryParsePatterns(paths); return tryLoadModuleUsingPaths(extensions, moduleName, baseDirectory, paths, pathPatterns, loader, /*onlyRecordFailures*/ false, state); } } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 3ee2a197e1bbd..53e0de202c9a4 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -12,7 +12,6 @@ import { OptionsNameMap, PackageJsonInfo, PackageJsonInfoCache, - ParsedPatterns, Pattern, SymlinkCache, ThisContainer, @@ -7513,7 +7512,6 @@ export interface ConfigFileSpecs { validatedFilesSpecBeforeSubstitution: readonly string[] | undefined; validatedIncludeSpecsBeforeSubstitution: readonly string[] | undefined; validatedExcludeSpecsBeforeSubstitution: readonly string[] | undefined; - pathPatterns: ParsedPatterns | undefined; isDefaultIncludeSpec: boolean; } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 48785426c60bd..560fc7b90ac28 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10020,13 +10020,10 @@ const parsedPatternsCache = new WeakMap, ParsedPatterns>(); * * @internal */ -export function tryParsePatterns(paths: MapLike, shouldCache: boolean = true, sortByAggregateLength: boolean = false): ParsedPatterns { - let result: ParsedPatterns | undefined; - if (shouldCache) { - result = parsedPatternsCache.get(paths); - if (result !== undefined) { - return result; - } +export function tryParsePatterns(paths: MapLike, sortByAggregateLength: boolean = false): ParsedPatterns { + let result = parsedPatternsCache.get(paths); + if (result !== undefined) { + return result; } let matchableStringSet: Set | undefined; @@ -10054,14 +10051,13 @@ export function tryParsePatterns(paths: MapLike, shouldCache: boolean return prefixComparison; }); - result = { - matchableStringSet, - sortedPatterns, - }; - - if (shouldCache) { - parsedPatternsCache.set(paths, result); - } + parsedPatternsCache.set( + paths, + result = { + matchableStringSet, + sortedPatterns, + }, + ); return result; } From 12d1a11f6f4838fdba728ab4e28dd9d7bf55ed99 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 28 Jun 2024 19:41:56 +0000 Subject: [PATCH 09/13] Swap conditions. --- src/compiler/core.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index f6fc7e6f9bcab..f75b504b40c2f 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -2540,7 +2540,7 @@ export function findBestPatternMatch(values: readonly T[], getPattern: (value for (let i = 0; i < endIndex; i++) { const v = values[i]; const pattern = getPattern(v); - if (isPatternMatch(pattern, candidate) && pattern.prefix.length > longestMatchPrefixLength) { + if (pattern.prefix.length > longestMatchPrefixLength && isPatternMatch(pattern, candidate)) { longestMatchPrefixLength = pattern.prefix.length; matchedValue = v; } From 012259e92ae363179af1212c38b3a1eb3b7509e4 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 28 Jun 2024 19:44:34 +0000 Subject: [PATCH 10/13] Always do a linear search. --- src/compiler/core.ts | 4 +-- src/compiler/utilities.ts | 57 +-------------------------------------- 2 files changed, 3 insertions(+), 58 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index f75b504b40c2f..3fb3010769809 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -2532,12 +2532,12 @@ export function matchedText(pattern: Pattern, candidate: string): string { * * @internal */ -export function findBestPatternMatch(values: readonly T[], getPattern: (value: T) => Pattern, candidate: string, endIndex: number = values.length): T | undefined { +export function findBestPatternMatch(values: readonly T[], getPattern: (value: T) => Pattern, candidate: string): T | undefined { let matchedValue: T | undefined; // use length of prefix as betterness criteria let longestMatchPrefixLength = -1; - for (let i = 0; i < endIndex; i++) { + for (let i = 0; i < values.length; i++) { const v = values[i]; const pattern = getPattern(v); if (pattern.prefix.length > longestMatchPrefixLength && isPatternMatch(pattern, candidate)) { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 560fc7b90ac28..591e5be1644dc 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -28,7 +28,6 @@ import { BarBarEqualsToken, BinaryExpression, binarySearch, - binarySearchKey, BindableObjectDefinePropertyCall, BindableStaticAccessExpression, BindableStaticElementAccessExpression, @@ -340,7 +339,6 @@ import { isParameterPropertyDeclaration, isParenthesizedExpression, isParenthesizedTypeNode, - isPatternMatch, isPrefixUnaryExpression, isPrivateIdentifier, isPropertyAccessExpression, @@ -10133,60 +10131,7 @@ export function matchPatternOrExact(patternOrStrings: ParsedPatterns, candidate: return undefined; } - let index = binarySearchKey(sortedPatterns, candidate, getPatternPrefix, compareStringsCaseSensitive); - if (index < 0) { - index = ~index; - } - - if (index >= sortedPatterns.length) { - // If we are past the end of the array, then the candidate length just exceeds the last prefix. - // Bump us back into a reasonable range. - index--; - } - - // `sortedPatterns` is sorted by prefixes, where longer prefixes should occur later; - // however, the sort is stable, so the original input order of patterns is preserved within each group. - // So for something like - // - // { prefix: "foo/", suffix: "bar"}, { "prefix: "", suffix: "" }, { prefix: "foo/", suffix: "" }, { "prefix: "", suffix: "bar" }, - // - // we will end up with - // - // { "prefix: "", suffix: "" }, { "prefix: "", suffix: "bar" }, { prefix: "foo/", suffix: "bar"}, { prefix: "foo/", suffix: "" } - // - // guaranteeing that within a group, the first match is ideal. - // - // Now the binary search may have landed us in the very middle of a group. If we are searching for "foo/" in - // - // ..., { prefix: "foo/", suffix: "" }, { prefix: "foo/", suffix: "my-suffix" }, ... - // - // then we could have ended up on an exact match. Keep walking backwards on exact matches until we find the first. - // This will allow us to try out all patterns with an identical prefix, while maintaining the relative original order of the - // patterns as specified. - const groupPrefix = sortedPatterns[index].prefix; - while (index > 0 && groupPrefix === sortedPatterns[index - 1].prefix) { - index--; - } - - for (let i = index; i < sortedPatterns.length; i++) { - const currentPattern = sortedPatterns[i]; - if (currentPattern.prefix !== groupPrefix) { - break; - } - - if (isPatternMatch(currentPattern, candidate)) { - return currentPattern; - } - } - - // We could not find a pattern within the group that matched. - // Technically we could walk backwards from here and find likely matches. - // For now, we'll just do a simple linear search up to the current group index. - return findBestPatternMatch(sortedPatterns, _ => _, candidate, /*endIndex*/ index); -} - -function getPatternPrefix(pattern: Pattern) { - return pattern.prefix; + return findBestPatternMatch(sortedPatterns, _ => _, candidate); } /** @internal */ From 94b522acfd587562931c15bd66b0735c3e5d66c4 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 11 Jul 2024 22:19:06 +0000 Subject: [PATCH 11/13] Just separate patterns into exact strings and maches. Remove all sorting logic. --- src/compiler/moduleNameResolver.ts | 3 --- src/compiler/utilities.ts | 29 ++++++++++------------------- 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 5291540dee4e4..46bda7af60046 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -1568,7 +1568,6 @@ function tryLoadModuleUsingPathsIfEligible(extensions: Extensions, moduleName: s trace(state.host, Diagnostics.paths_option_is_specified_looking_for_a_pattern_to_match_module_name_0, moduleName); } const baseDirectory = getPathsBasePath(state.compilerOptions, state.host)!; // Always defined when 'paths' is defined - // TODO: do we need to sort by aggregate length? const pathPatterns = tryParsePatterns(paths); return tryLoadModuleUsingPaths(extensions, moduleName, baseDirectory, paths, pathPatterns, loader, /*onlyRecordFailures*/ false, state); } @@ -2525,7 +2524,6 @@ function loadNodeModuleFromDirectoryWorker(extensions: Extensions, candidate: st if (state.traceEnabled) { trace(state.host, Diagnostics.package_json_has_a_typesVersions_entry_0_that_matches_compiler_version_1_looking_for_a_pattern_to_match_module_name_2, versionPaths.version, version, moduleName); } - // TODO: do we need to sort by aggregate length? const pathPatterns = tryParsePatterns(versionPaths.paths); const result = tryLoadModuleUsingPaths(extensions, moduleName, candidate, versionPaths.paths, pathPatterns, loader, onlyRecordFailuresForPackageFile || onlyRecordFailuresForIndex, state); if (result) { @@ -3121,7 +3119,6 @@ function loadModuleFromSpecificNodeModulesDirectory(extensions: Extensions, modu trace(state.host, Diagnostics.package_json_has_a_typesVersions_entry_0_that_matches_compiler_version_1_looking_for_a_pattern_to_match_module_name_2, versionPaths.version, version, rest); } const packageDirectoryExists = nodeModulesDirectoryExists && directoryProbablyExists(packageDirectory, state.host); - // TODO: do we need to sort by aggregate length? const pathPatterns = tryParsePatterns(versionPaths.paths); const fromPaths = tryLoadModuleUsingPaths(extensions, rest, packageDirectory, versionPaths.paths, pathPatterns, loader, !packageDirectoryExists, state); if (fromPaths) { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 591e5be1644dc..424d21d1bdfb6 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10006,26 +10006,25 @@ export function tryParsePattern(pattern: string): string | Pattern | undefined { /** @internal */ export interface ParsedPatterns { matchableStringSet: ReadonlySet | undefined; - sortedPatterns: (readonly Pattern[]) | undefined; + patterns: (readonly Pattern[]) | undefined; } const parsedPatternsCache = new WeakMap, ParsedPatterns>(); /** - * Divides patterns into a set of exact specifiers and sorted patterns. - * NOTE that this function caches, and assumes the same `paths` argument will - * never be provided with a different value for `sortByAggregateLength`. + * Divides patterns into a set of exact specifiers and patterns. + * NOTE that this function caches the result based on object identity. * * @internal */ -export function tryParsePatterns(paths: MapLike, sortByAggregateLength: boolean = false): ParsedPatterns { +export function tryParsePatterns(paths: MapLike): ParsedPatterns { let result = parsedPatternsCache.get(paths); if (result !== undefined) { return result; } let matchableStringSet: Set | undefined; - let sortedPatterns: Pattern[] | undefined; + let patterns: Pattern[] | undefined; const pathList = getOwnKeys(paths); for (const path of pathList) { @@ -10037,23 +10036,15 @@ export function tryParsePatterns(paths: MapLike, sortByAggregateLength (matchableStringSet ??= new Set()).add(patternOrStr); } else { - (sortedPatterns ??= []).push(patternOrStr); + (patterns ??= []).push(patternOrStr); } } - sortedPatterns?.sort((a, b) => { - const prefixComparison = compareStringsCaseSensitive(a.prefix, b.prefix); - if (prefixComparison === 0 && sortByAggregateLength) { - return a.suffix.length - b.suffix.length; - } - return prefixComparison; - }); - parsedPatternsCache.set( paths, result = { matchableStringSet, - sortedPatterns, + patterns, }, ); @@ -10121,17 +10112,17 @@ export const emptyFileSystemEntries: FileSystemEntries = { * @internal */ export function matchPatternOrExact(patternOrStrings: ParsedPatterns, candidate: string): string | Pattern | undefined { - const { matchableStringSet, sortedPatterns } = patternOrStrings; + const { matchableStringSet, patterns } = patternOrStrings; if (matchableStringSet?.has(candidate)) { return candidate; } - if (sortedPatterns === undefined || sortedPatterns.length === 0) { + if (patterns === undefined || patterns.length === 0) { return undefined; } - return findBestPatternMatch(sortedPatterns, _ => _, candidate); + return findBestPatternMatch(patterns, _ => _, candidate); } /** @internal */ From 5342b46092fedbb59f542526ecc085420e5828f6 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Wed, 14 Aug 2024 14:54:11 -0700 Subject: [PATCH 12/13] Addressed feedback. --- src/compiler/utilities.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 424d21d1bdfb6..bcdf514eacdf5 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10105,14 +10105,15 @@ export const emptyFileSystemEntries: FileSystemEntries = { }; /** - * patternOrStrings contains both patterns (containing "*") and regular strings. + * `parsedPatterns` contains both patterns (containing "*") and regular strings. * Return an exact match if possible, or a pattern match, or undefined. * (These are verified by verifyCompilerOptions to have 0 or 1 "*" characters.) * * @internal */ -export function matchPatternOrExact(patternOrStrings: ParsedPatterns, candidate: string): string | Pattern | undefined { - const { matchableStringSet, patterns } = patternOrStrings; +export function matchPatternOrExact(parsedPatterns: ParsedPatterns, candidate: string): string | Pattern | undefined { + const { matchableStringSet, patterns } = parsedPatterns; + if (matchableStringSet?.has(candidate)) { return candidate; From 88c76adfe8cc7f4bf8b39c976320e27875865830 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Wed, 28 Aug 2024 20:08:56 +0000 Subject: [PATCH 13/13] Format. --- src/compiler/utilities.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 1160573ff6ab4..673c0b38c156d 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -10082,7 +10082,6 @@ export const emptyFileSystemEntries: FileSystemEntries = { export function matchPatternOrExact(parsedPatterns: ParsedPatterns, candidate: string): string | Pattern | undefined { const { matchableStringSet, patterns } = parsedPatterns; - if (matchableStringSet?.has(candidate)) { return candidate; }