Skip to content

Remove refCount from resolutions as we dont need it explicitly since its tracked by files it references #59041

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.

Sign up for GitHub

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
sheetalkamat merged 1 commit into from
Jun 26, 2024
Merged

Remove refCount from resolutions as we dont need it explicitly since its tracked by files it references #59041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 16 additions & 26 deletions src/compiler/resolutionCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,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 @@ -1107,28 +1106,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 @@ -1156,7 +1148,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 @@ -1177,7 +1169,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 @@ -1393,10 +1385,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
View file
Open in desktop
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