Skip to content

Commit

Permalink
Remove refCount from resolutions as we dont need it explicitly since …
Browse files Browse the repository at this point in the history
…its tracked by files it references (#59041)
  • Loading branch information
sheetalkamat committed Jun 26, 2024
1 parent 7c011e7 commit a6bc4ec
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 37 deletions.
42 changes: 16 additions & 26 deletions src/compiler/resolutionCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ export interface ResolutionWithFailedLookupLocations {
failedLookupLocations?: string[];
affectingLocations?: string[];
isInvalidated?: boolean;
refCount?: number;
// Files that have this resolution using
files?: Set<Path>;
alternateResult?: string;
Expand Down Expand Up @@ -1095,28 +1094,21 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
getResolutionWithResolvedFileName: GetResolutionWithResolvedFileName<T, R>,
deferWatchingNonRelativeResolution: boolean,
) {
if (resolution.refCount) {
resolution.refCount++;
Debug.assertIsDefined(resolution.files);
(resolution.files ??= new Set()).add(filePath);
if (resolution.files.size !== 1) return;
if (!deferWatchingNonRelativeResolution || isExternalModuleNameRelative(name)) {
watchFailedLookupLocationOfResolution(resolution);
}
else {
resolution.refCount = 1;
Debug.assert(!resolution.files?.size); // This resolution shouldnt be referenced by any file yet
if (!deferWatchingNonRelativeResolution || isExternalModuleNameRelative(name)) {
watchFailedLookupLocationOfResolution(resolution);
}
else {
nonRelativeExternalModuleResolutions.add(name, resolution);
}
const resolved = getResolutionWithResolvedFileName(resolution);
if (resolved && resolved.resolvedFileName) {
const key = resolutionHost.toPath(resolved.resolvedFileName);
let resolutions = resolvedFileToResolution.get(key);
if (!resolutions) resolvedFileToResolution.set(key, resolutions = new Set());
resolutions.add(resolution);
}
nonRelativeExternalModuleResolutions.add(name, resolution);
}
const resolved = getResolutionWithResolvedFileName(resolution);
if (resolved && resolved.resolvedFileName) {
const key = resolutionHost.toPath(resolved.resolvedFileName);
let resolutions = resolvedFileToResolution.get(key);
if (!resolutions) resolvedFileToResolution.set(key, resolutions = new Set());
resolutions.add(resolution);
}
(resolution.files ??= new Set()).add(filePath);
}

function watchFailedLookupLocation(failedLookupLocation: string, setAtRoot: boolean) {
Expand Down Expand Up @@ -1144,7 +1136,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
}

function watchFailedLookupLocationOfResolution(resolution: ResolutionWithFailedLookupLocations) {
Debug.assert(!!resolution.refCount);
Debug.assert(!!resolution.files?.size);

const { failedLookupLocations, affectingLocations, alternateResult } = resolution;
if (!failedLookupLocations?.length && !affectingLocations?.length && !alternateResult) return;
Expand All @@ -1165,7 +1157,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
}

function watchAffectingLocationsOfResolution(resolution: ResolutionWithFailedLookupLocations, addToResolutionsWithOnlyAffectingLocations: boolean) {
Debug.assert(!!resolution.refCount);
Debug.assert(!!resolution.files?.size);
const { affectingLocations } = resolution;
if (!affectingLocations?.length) return;
if (addToResolutionsWithOnlyAffectingLocations) resolutionsWithOnlyAffectingLocations.add(resolution);
Expand Down Expand Up @@ -1381,10 +1373,8 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
syncDirWatcherRemove?: boolean,
) {
Debug.checkDefined(resolution.files).delete(filePath);
resolution.refCount!--;
if (resolution.refCount) {
return;
}
if (resolution.files!.size) return;
resolution.files = undefined;
const resolved = getResolutionWithResolvedFileName(resolution);
if (resolved && resolved.resolvedFileName) {
const key = resolutionHost.toPath(resolved.resolvedFileName);
Expand Down
17 changes: 6 additions & 11 deletions src/harness/incrementalUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,15 +254,11 @@ export function verifyResolutionCache(
// Verify ref count
resolutionToRefs.forEach((info, resolution) => {
ts.Debug.assert(
resolution.refCount === info.length,
`${projectName}:: Expected Resolution ref count ${info.length} but got ${resolution.refCount}`,
resolution.files?.size === info.length,
`${projectName}:: Expected Resolution ref count ${info.length} but got ${resolution.files?.size}`,
() =>
`Expected from:: ${JSON.stringify(info, undefined, " ")}` +
`Actual from: ${resolution.refCount}`,
);
ts.Debug.assert(
resolutionToExpected.get(resolution)!.refCount === resolution.refCount,
`${projectName}:: Expected Resolution ref count ${resolutionToExpected.get(resolution)!.refCount} but got ${resolution.refCount}`,
`Actual from: ${resolution.files?.size}`,
);
verifySet(resolutionToExpected.get(resolution)!.files, resolution.files, `${projectName}:: Resolution files`);
});
Expand All @@ -280,10 +276,9 @@ export function verifyResolutionCache(
actual.resolvedTypeReferenceDirectives.forEach((_resolutions, path) => expected.removeResolutionsOfFile(path));
expected.finishCachingPerDirectoryResolution(/*newProgram*/ undefined, actualProgram);

resolutionToExpected.forEach(expected => {
ts.Debug.assert(!expected.refCount, `${projectName}:: All the resolution should be released`);
ts.Debug.assert(!expected.files?.size, `${projectName}:: Shouldnt ref to any files`);
});
resolutionToExpected.forEach(
expected => ts.Debug.assert(!expected.files?.size, `${projectName}:: Shouldnt ref to any files`),
);
ts.Debug.assert(expected.resolvedFileToResolution.size === 0, `${projectName}:: resolvedFileToResolution should be released`);
ts.Debug.assert(expected.resolutionsWithFailedLookups.size === 0, `${projectName}:: resolutionsWithFailedLookups should be released`);
ts.Debug.assert(expected.resolutionsWithOnlyAffectingLocations.size === 0, `${projectName}:: resolutionsWithOnlyAffectingLocations should be released`);
Expand Down

0 comments on commit a6bc4ec

Please sign in to comment.