Skip to content

Reuse program structure #3616

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

Closed
wants to merge 12 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 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
3 changes: 2 additions & 1 deletion Jakefile.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ var harnessSources = [
"services/patternMatcher.ts",
"versionCache.ts",
"convertToBase64.ts",
"transpile.ts"
"transpile.ts",
"reuseProgramStructure.ts"
].map(function (f) {
return path.join(unittestsDirectory, f);
})).concat([
Expand Down
21 changes: 6 additions & 15 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -892,28 +892,19 @@ namespace ts {
// Escape the name in the "require(...)" clause to ensure we find the right symbol.
let moduleName = escapeIdentifier(moduleReferenceLiteral.text);

if (!moduleName) return;
if (!moduleName) {
return;
}
let isRelative = isExternalModuleNameRelative(moduleName);
if (!isRelative) {
let symbol = getSymbol(globals, '"' + moduleName + '"', SymbolFlags.ValueModule);
if (symbol) {
return symbol;
}
}
let fileName: string;
let sourceFile: SourceFile;
while (true) {
fileName = normalizePath(combinePaths(searchPath, moduleName));
sourceFile = forEach(supportedExtensions, extension => host.getSourceFile(fileName + extension));
if (sourceFile || isRelative) {
break;
}
let parentPath = getDirectoryPath(searchPath);
if (parentPath === searchPath) {
break;
}
searchPath = parentPath;
}

let fileName = getResolvedModuleFileName(getSourceFile(location), moduleReferenceLiteral);
let sourceFile = fileName && host.getSourceFile(fileName);
if (sourceFile) {
if (sourceFile.symbol) {
return sourceFile.symbol;
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5634,7 +5634,7 @@ namespace ts {
// will immediately bail out of walking any subtrees when we can see that their parents
// are already correct.
let result = Parser.parseSourceFile(sourceFile.fileName, newText, sourceFile.languageVersion, syntaxCursor, /* setParentNode */ true)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unnecessary change, will remove it from the PR

return result;
}

Expand Down
267 changes: 215 additions & 52 deletions src/compiler/program.ts

Large diffs are not rendered by default.

15 changes: 13 additions & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1176,8 +1176,11 @@ namespace ts {
// Stores a line map for the file.
// This field should never be used directly to obtain line map, use getLineMap function instead.
/* @internal */ lineMap: number[];

/* @internal */ classifiableNames?: Map<string>;
// Stores a mapping 'external module reference text' -> 'resolved file name' | undefined
// Content of this fiels should never be used directly - use getResolvedModuleFileName/setResolvedModuleFileName functions instead
/* @internal */ resolvedModules: Map<string>;
/* @internal */ imports: LiteralExpression[];
}

export interface ScriptReferenceHost {
Expand All @@ -1195,6 +1198,12 @@ namespace ts {
}

export interface Program extends ScriptReferenceHost {

/**
* Get a list of root file names that were passed to a 'createProgram'
*/
getRootFileNames(): string[]

/**
* Get a list of files in the program
*/
Expand Down Expand Up @@ -1235,6 +1244,7 @@ namespace ts {
/* @internal */ getIdentifierCount(): number;
/* @internal */ getSymbolCount(): number;
/* @internal */ getTypeCount(): number;
/* @internal */ structureIsReused?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment that this is for tests ony.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

export interface SourceMapSpan {
Expand Down Expand Up @@ -1881,7 +1891,7 @@ namespace ts {
CarriageReturnLineFeed = 0,
LineFeed = 1,
}
export interface LineAndCharacter {
line: number;
/*
Expand Down Expand Up @@ -2065,6 +2075,7 @@ namespace ts {
getCanonicalFileName(fileName: string): string;
useCaseSensitiveFileNames(): boolean;
getNewLine(): string;
hasChanges?(oldFile: SourceFile): boolean;
}

export interface TextSpan {
Expand Down
34 changes: 34 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,40 @@ namespace ts {
return node.end - node.pos;
}

export function arrayIsEqualTo<T>(arr1: T[], arr2: T[], comparer: (a: T, b: T) => boolean): boolean {
if (!arr1 || !arr2) {
return arr1 === arr2;
}

if (arr1.length !== arr2.length) {
return false;
}

for (let i = 0; i < arr1.length; ++i) {
if (!comparer(arr1[i], arr2[i])) {
return false;
}
}

return true;
}

export function hasResolvedModuleName(sourceFile: SourceFile, moduleName: LiteralExpression): boolean {
return sourceFile.resolvedModules && hasProperty(sourceFile.resolvedModules, moduleName.text);
}

export function getResolvedModuleFileName(sourceFile: SourceFile, moduleName: LiteralExpression): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we make this take moduleNameText instead of a node, this way it is easier to understand the logic looking at the call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

return sourceFile.resolvedModules && sourceFile.resolvedModules[moduleName.text];
Copy link
Contributor

Choose a reason for hiding this comment

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

should not this be hasOwnProperty to avoid modulename looks for something like "toString".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

never mind. noticed that we are using undefined to mark missing.

}

export function setResolvedModuleName(sourceFile: SourceFile, moduleName: LiteralExpression, resolvedFileName: string): void {
if (!sourceFile.resolvedModules) {
sourceFile.resolvedModules = {};
}

sourceFile.resolvedModules[moduleName.text] = resolvedFileName;
}

// Returns true if this node contains a parse error anywhere underneath it.
export function containsParseError(node: Node): boolean {
aggregateChildData(node);
Expand Down
10 changes: 7 additions & 3 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,8 @@ namespace ts {
public languageVersion: ScriptTarget;
public identifiers: Map<string>;
public nameTable: Map<string>;

public resolvedModules: Map<string>;
public imports: LiteralExpression[];
private namedDeclarations: Map<Declaration[]>;

public update(newText: string, textChangeRange: TextChangeRange): SourceFile {
Expand Down Expand Up @@ -2471,6 +2472,8 @@ namespace ts {
let newSettings = hostCache.compilationSettings();
let changesInCompilationSettingsAffectSyntax = oldSettings && oldSettings.target !== newSettings.target;

let reusableOldProgram = changesInCompilationSettingsAffectSyntax ? undefined : program;
Copy link
Contributor

Choose a reason for hiding this comment

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

we can move the target check to create program right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure


// Now create a new compiler
let newProgram = createProgram(hostCache.getRootFileNames(), newSettings, {
getSourceFile: getOrCreateSourceFile,
Expand All @@ -2480,8 +2483,9 @@ namespace ts {
getNewLine: () => host.getNewLine ? host.getNewLine() : "\r\n",
getDefaultLibFileName: (options) => host.getDefaultLibFileName(options),
writeFile: (fileName, data, writeByteOrderMark) => { },
getCurrentDirectory: () => host.getCurrentDirectory()
});
getCurrentDirectory: () => host.getCurrentDirectory(),
hasChanges: oldFile => oldFile.version !== hostCache.getVersion(oldFile.fileName)
}, reusableOldProgram);

// Release any files we have acquired in the old program but are
// not part of the new program.
Expand Down
Loading