Skip to content

When building projects in watch mode, only schedule projects that need build or need update to bundle #48865

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
merged 9 commits into from
May 23, 2022
Merged

When building projects in watch mode, only schedule projects that need build or need update to bundle #48865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
24a8aff
Handle timesouts to reflect the time it was set with
sheetalkamat Apr 27, 2022
ac151af
Remove unused internal calls from solution builder
sheetalkamat Apr 27, 2022
ab3d2d4
If the project doesnt need building or updating bundle, dont schedule…
sheetalkamat Apr 27, 2022
9ceb1db
Reduce the time between project builds to 100ms
sheetalkamat Apr 27, 2022
568e4e5
Some tests for projects building
sheetalkamat Apr 28, 2022
0e9ab75
Schedule builds such that when change is not detected 5 projects are …
sheetalkamat Apr 28, 2022
55c8f40
Merge branch 'main' into watchTimes
sheetalkamat May 5, 2022
95a94da
Fix tests in main
sheetalkamat May 5, 2022
37dee49
Merge branch 'main' into watchTimes
sheetalkamat May 5, 2022
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
116 changes: 78 additions & 38 deletions src/compiler/tsbuildPublic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,6 @@ namespace ts {
// Testing only
/*@internal*/ getUpToDateStatusOfProject(project: string): UpToDateStatus;
/*@internal*/ invalidateProject(configFilePath: ResolvedConfigFilePath, reloadLevel?: ConfigFileProgramReloadLevel): void;
/*@internal*/ buildNextInvalidatedProject(): void;
/*@internal*/ getAllParsedConfigs(): readonly ParsedCommandLine[];
/*@internal*/ close(): void;
}

Expand Down Expand Up @@ -250,7 +248,6 @@ namespace ts {
allProjectBuildPending: boolean;
needsSummary: boolean;
watchAllProjectsPending: boolean;
currentInvalidatedProject: InvalidatedProject<T> | undefined;

// Watch state
readonly watch: boolean;
Expand Down Expand Up @@ -332,7 +329,6 @@ namespace ts {
allProjectBuildPending: true,
needsSummary: true,
watchAllProjectsPending: watch,
currentInvalidatedProject: undefined,

// Watch state
watch,
Expand Down Expand Up @@ -684,7 +680,6 @@ namespace ts {
projectPath: ResolvedConfigFilePath
) {
state.projectPendingBuild.delete(projectPath);
state.currentInvalidatedProject = undefined;
return state.diagnostics.has(projectPath) ?
ExitStatus.DiagnosticsPresent_OutputsSkipped :
ExitStatus.Success;
Expand Down Expand Up @@ -1171,19 +1166,22 @@ namespace ts {
!isIncrementalCompilation(config.options);
}

function getNextInvalidatedProject<T extends BuilderProgram>(
interface InvalidateProjectCreateInfo {
kind: InvalidatedProjectKind;
status: UpToDateStatus;
project: ResolvedConfigFileName;
projectPath: ResolvedConfigFilePath;
projectIndex: number;
config: ParsedCommandLine;
}

function getNextInvalidatedProjectCreateInfo<T extends BuilderProgram>(
state: SolutionBuilderState<T>,
buildOrder: AnyBuildOrder,
reportQueue: boolean
): InvalidatedProject<T> | undefined {
): InvalidateProjectCreateInfo | undefined {
if (!state.projectPendingBuild.size) return undefined;
if (isCircularBuildOrder(buildOrder)) return undefined;
if (state.currentInvalidatedProject) {
// Only if same buildOrder the currentInvalidated project can be sent again
return arrayIsEqualTo(state.currentInvalidatedProject.buildOrder, buildOrder) ?
state.currentInvalidatedProject :
undefined;
}

const { options, projectPendingBuild } = state;
for (let projectIndex = 0; projectIndex < buildOrder.length; projectIndex++) {
Expand Down Expand Up @@ -1220,9 +1218,9 @@ namespace ts {
}

const status = getUpToDateStatus(state, config, projectPath);
verboseReportProjectStatus(state, project, status);
if (!options.force) {
if (status.type === UpToDateStatusType.UpToDate) {
verboseReportProjectStatus(state, project, status);
reportAndStoreErrors(state, projectPath, getConfigFileParsingDiagnostics(config));
projectPendingBuild.delete(projectPath);
// Up to date, skip
Expand All @@ -1235,17 +1233,19 @@ namespace ts {

if (status.type === UpToDateStatusType.UpToDateWithUpstreamTypes) {
reportAndStoreErrors(state, projectPath, getConfigFileParsingDiagnostics(config));
return createUpdateOutputFileStampsProject(
state,
return {
kind: InvalidatedProjectKind.UpdateOutputFileStamps,
status,
project,
projectPath,
config,
buildOrder
);
projectIndex,
config
};
}
}

if (status.type === UpToDateStatusType.UpstreamBlocked) {
verboseReportProjectStatus(state, project, status);
reportAndStoreErrors(state, projectPath, getConfigFileParsingDiagnostics(config));
projectPendingBuild.delete(projectPath);
if (options.verbose) {
Expand All @@ -1262,28 +1262,63 @@ namespace ts {
}

if (status.type === UpToDateStatusType.ContainerOnly) {
verboseReportProjectStatus(state, project, status);
reportAndStoreErrors(state, projectPath, getConfigFileParsingDiagnostics(config));
projectPendingBuild.delete(projectPath);
// Do nothing
continue;
}

return createBuildOrUpdateInvalidedProject(
needsBuild(state, status, config) ?
return {
kind: needsBuild(state, status, config) ?
InvalidatedProjectKind.Build :
InvalidatedProjectKind.UpdateBundle,
state,
status,
project,
projectPath,
projectIndex,
config,
buildOrder,
);
};
}

return undefined;
}

function createInvalidatedProjectWithInfo<T extends BuilderProgram>(
state: SolutionBuilderState<T>,
info: InvalidateProjectCreateInfo,
buildOrder: AnyBuildOrder,
) {
verboseReportProjectStatus(state, info.project, info.status);
return info.kind !== InvalidatedProjectKind.UpdateOutputFileStamps ?
createBuildOrUpdateInvalidedProject(
info.kind,
state,
info.project,
info.projectPath,
info.projectIndex,
info.config,
buildOrder as BuildOrder,
) :
createUpdateOutputFileStampsProject(
state,
info.project,
info.projectPath,
info.config,
buildOrder as BuildOrder
);
}

function getNextInvalidatedProject<T extends BuilderProgram>(
state: SolutionBuilderState<T>,
buildOrder: AnyBuildOrder,
reportQueue: boolean
): InvalidatedProject<T> | undefined {
const info = getNextInvalidatedProjectCreateInfo(state, buildOrder, reportQueue);
if (!info) return info;
return createInvalidatedProjectWithInfo(state, info, buildOrder);
}

function listEmittedFile({ write }: SolutionBuilderState, proj: ParsedCommandLine, file: string) {
if (write && proj.options.listEmittedFiles) {
write(`TSFILE: ${file}`);
Expand Down Expand Up @@ -1769,37 +1804,47 @@ namespace ts {
function invalidateProjectAndScheduleBuilds(state: SolutionBuilderState, resolvedPath: ResolvedConfigFilePath, reloadLevel: ConfigFileProgramReloadLevel) {
state.reportFileChangeDetected = true;
invalidateProject(state, resolvedPath, reloadLevel);
scheduleBuildInvalidatedProject(state);
scheduleBuildInvalidatedProject(state, 250, /*changeDetected*/ true);
}

function scheduleBuildInvalidatedProject(state: SolutionBuilderState) {
function scheduleBuildInvalidatedProject(state: SolutionBuilderState, time: number, changeDetected: boolean) {
const { hostWithWatch } = state;
if (!hostWithWatch.setTimeout || !hostWithWatch.clearTimeout) {
return;
}
if (state.timerToBuildInvalidatedProject) {
hostWithWatch.clearTimeout(state.timerToBuildInvalidatedProject);
}
state.timerToBuildInvalidatedProject = hostWithWatch.setTimeout(buildNextInvalidatedProject, 250, state);
state.timerToBuildInvalidatedProject = hostWithWatch.setTimeout(buildNextInvalidatedProject, time, state, changeDetected);
}

function buildNextInvalidatedProject(state: SolutionBuilderState) {
function buildNextInvalidatedProject(state: SolutionBuilderState, changeDetected: boolean) {
state.timerToBuildInvalidatedProject = undefined;
if (state.reportFileChangeDetected) {
state.reportFileChangeDetected = false;
state.projectErrorsReported.clear();
reportWatchStatus(state, Diagnostics.File_change_detected_Starting_incremental_compilation);
}
let projectsBuilt = 0;
const buildOrder = getBuildOrder(state);
const invalidatedProject = getNextInvalidatedProject(state, buildOrder, /*reportQueue*/ false);
if (invalidatedProject) {
invalidatedProject.done();
if (state.projectPendingBuild.size) {
// Schedule next project for build
if (state.watch && !state.timerToBuildInvalidatedProject) {
scheduleBuildInvalidatedProject(state);
projectsBuilt++;
while (state.projectPendingBuild.size) {
// If already scheduled, skip
if (state.timerToBuildInvalidatedProject) return;
// Before scheduling check if the next project needs build
const info = getNextInvalidatedProjectCreateInfo(state, buildOrder, /*reportQueue*/ false);
if (!info) break; // Nothing to build any more
if (info.kind !== InvalidatedProjectKind.UpdateOutputFileStamps && (changeDetected || projectsBuilt === 5)) {
Copy link
Member

Choose a reason for hiding this comment

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

How did you choose 5? What’s the advantage of having a delay between groups at all? (Is it so there are points where the operation can be canceled if a new change comes in?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I assumed that on an average the change to build 5 projects might be couple of seconds or less and that would be good time to check if there is new change...

Without adding delay, we wont be able to check if new change is detected and cancel existing chain of build

// Schedule next project for build
scheduleBuildInvalidatedProject(state, 100, /*changeDetected*/ false);
return;
}
return;
const project = createInvalidatedProjectWithInfo(state, info, buildOrder);
project.done();
if (info.kind !== InvalidatedProjectKind.UpdateOutputFileStamps) projectsBuilt++;
}
}
disableCache(state);
Expand Down Expand Up @@ -1961,11 +2006,6 @@ namespace ts {
return getUpToDateStatus(state, parseConfigFile(state, configFileName, configFilePath), configFilePath);
},
invalidateProject: (configFilePath, reloadLevel) => invalidateProject(state, configFilePath, reloadLevel || ConfigFileProgramReloadLevel.None),
buildNextInvalidatedProject: () => buildNextInvalidatedProject(state),
getAllParsedConfigs: () => arrayFrom(mapDefinedIterator(
state.configFileCache.values(),
config => isParsedCommandLine(config) ? config : undefined
)),
close: () => stopWatching(state),
};
}
Expand Down
49 changes: 38 additions & 11 deletions src/harness/virtualFileSystemWithWatch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,18 +279,27 @@ interface Array<T> { length: number; [n: number]: T; }`
}
}

interface CallbackData {
cb: TimeOutCallback;
args: any[];
ms: number | undefined;
time: number;
}
class Callbacks {
private map: TimeOutCallback[] = [];
private map: { cb: TimeOutCallback; args: any[]; ms: number | undefined; time: number; }[] = [];
private nextId = 1;

constructor(private host: TestServerHost) {
}

getNextId() {
return this.nextId;
}

register(cb: (...args: any[]) => void, args: any[]) {
register(cb: TimeOutCallback, args: any[], ms?: number) {
const timeoutId = this.nextId;
this.nextId++;
this.map[timeoutId] = cb.bind(/*this*/ undefined, ...args);
this.map[timeoutId] = { cb, args, ms, time: this.host.getTime() };
return timeoutId;
}

Expand All @@ -308,9 +317,19 @@ interface Array<T> { length: number; [n: number]: T; }`
return n;
}

private invokeCallback({ cb, args, ms, time }: CallbackData) {
if (ms !== undefined) {
const newTime = ms + time;
if (this.host.getTime() < newTime) {
this.host.setTime(newTime);
}
}
cb(...args);
}

invoke(invokeKey?: number) {
if (invokeKey) {
this.map[invokeKey]();
this.invokeCallback(this.map[invokeKey]);
delete this.map[invokeKey];
return;
}
Expand All @@ -319,13 +338,13 @@ interface Array<T> { length: number; [n: number]: T; }`
// so do not clear the entire callback list regardless. Only remove the
// ones we have invoked.
for (const key in this.map) {
this.map[key]();
this.invokeCallback(this.map[key]);
delete this.map[key];
}
}
}

type TimeOutCallback = () => any;
type TimeOutCallback = (...args: any[]) => void;

export interface TestFileWatcher {
cb: FileWatcherCallback;
Expand Down Expand Up @@ -380,8 +399,8 @@ interface Array<T> { length: number; [n: number]: T; }`
private time = timeIncrements;
getCanonicalFileName: (s: string) => string;
private toPath: (f: string) => Path;
private timeoutCallbacks = new Callbacks();
private immediateCallbacks = new Callbacks();
private timeoutCallbacks = new Callbacks(this);
private immediateCallbacks = new Callbacks(this);
readonly screenClears: number[] = [];

readonly watchedFiles = createMultiMap<Path, TestFileWatcher>();
Expand Down Expand Up @@ -478,6 +497,14 @@ interface Array<T> { length: number; [n: number]: T; }`
return new Date(this.time);
}

getTime() {
return this.time;
}

setTime(time: number) {
this.time = time;
}

private reloadFS(fileOrFolderOrSymLinkList: readonly FileOrFolderOrSymLink[], options?: Partial<ReloadWatchInvokeOptions>) {
Debug.assert(this.fs.size === 0);
fileOrFolderOrSymLinkList = fileOrFolderOrSymLinkList.concat(this.withSafeList ? safeList : []);
Expand Down Expand Up @@ -935,8 +962,8 @@ interface Array<T> { length: number; [n: number]: T; }`
}

// TOOD: record and invoke callbacks to simulate timer events
setTimeout(callback: TimeOutCallback, _time: number, ...args: any[]) {
return this.timeoutCallbacks.register(callback, args);
setTimeout(callback: TimeOutCallback, ms: number, ...args: any[]) {
return this.timeoutCallbacks.register(callback, args, ms);
}

getNextTimeoutId() {
Expand Down Expand Up @@ -980,7 +1007,7 @@ interface Array<T> { length: number; [n: number]: T; }`
this.immediateCallbacks.invoke();
}

setImmediate(callback: TimeOutCallback, _time: number, ...args: any[]) {
setImmediate(callback: TimeOutCallback, ...args: any[]) {
return this.immediateCallbacks.register(callback, args);
}

Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
"unittests/tsbuildWatch/noEmit.ts",
"unittests/tsbuildWatch/noEmitOnError.ts",
"unittests/tsbuildWatch/programUpdates.ts",
"unittests/tsbuildWatch/projectsBuilding.ts",
"unittests/tsbuildWatch/publicApi.ts",
"unittests/tsbuildWatch/reexport.ts",
"unittests/tsbuildWatch/watchEnvironment.ts",
Expand Down
4 changes: 2 additions & 2 deletions src/testRunner/unittests/tsbuild/sample.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,11 +338,11 @@ namespace ts {
function verifyInvalidation(heading: string) {
// Rebuild this project
builder.invalidateProject(logicConfig.path as ResolvedConfigFilePath);
builder.buildNextInvalidatedProject();
builder.getNextInvalidatedProject()?.done();
baselineState(`${heading}:: After rebuilding logicConfig`);

// Build downstream projects should update 'tests', but not 'core'
builder.buildNextInvalidatedProject();
builder.getNextInvalidatedProject()?.done();
baselineState(`${heading}:: After building next project`);
}

Expand Down
4 changes: 1 addition & 3 deletions src/testRunner/unittests/tsbuildWatch/demo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ namespace ts.tscWatch {
change: sys => sys.writeFile(coreFiles[0].path, coreFiles[0].content),
timeouts: sys => {
sys.checkTimeoutQueueLengthAndRun(1); // build core
sys.checkTimeoutQueueLengthAndRun(1); // build animals
sys.checkTimeoutQueueLengthAndRun(1); // build zoo
sys.checkTimeoutQueueLengthAndRun(1); // build solution
sys.checkTimeoutQueueLengthAndRun(1); // build animals, zoo and solution
sys.checkTimeoutQueueLength(0);
},
}
Expand Down
3 changes: 1 addition & 2 deletions src/testRunner/unittests/tsbuildWatch/moduleResolution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ namespace ts.tscWatch {
caption: "Append text",
change: sys => sys.appendFile(`${projectRoot}/project1/index.ts`, "const bar = 10;"),
timeouts: sys => {
sys.checkTimeoutQueueLengthAndRun(1); // build project1
sys.checkTimeoutQueueLengthAndRun(1); // Solution
sys.checkTimeoutQueueLengthAndRun(1); // build project1 and solution
sys.checkTimeoutQueueLength(0);
}
},
Expand Down
Loading