Skip to content

Make perfLogger potentially undefined rather than using a noop logger #53229

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

Merged
merged 2 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,9 +493,9 @@ const binder = /* @__PURE__ */ createBinder();
/** @internal */
export function bindSourceFile(file: SourceFile, options: CompilerOptions) {
performance.mark("beforeBind");
perfLogger.logStartBindFile("" + file.fileName);
perfLogger?.logStartBindFile("" + file.fileName);
binder(file, options);
perfLogger.logStopBindFile();
perfLogger?.logStopBindFile();
performance.mark("afterBind");
performance.measure("Bind", "beforeBind", "afterBind");
}
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1328,7 +1328,7 @@ export function resolveModuleName(moduleName: string, containingFile: string, co
}
}

perfLogger.logStartResolveModule(moduleName /* , containingFile, ModuleResolutionKind[moduleResolution]*/);
perfLogger?.logStartResolveModule(moduleName /* , containingFile, ModuleResolutionKind[moduleResolution]*/);
switch (moduleResolution) {
case ModuleResolutionKind.Node16:
result = node16ModuleNameResolver(moduleName, containingFile, compilerOptions, host, cache, redirectedReference, resolutionMode);
Expand All @@ -1348,8 +1348,8 @@ export function resolveModuleName(moduleName: string, containingFile: string, co
default:
return Debug.fail(`Unexpected moduleResolution: ${moduleResolution}`);
}
if (result && result.resolvedModule) perfLogger.logInfoEvent(`Module "${moduleName}" resolved to "${result.resolvedModule.resolvedFileName}"`);
perfLogger.logStopResolveModule((result && result.resolvedModule) ? "" + result.resolvedModule.resolvedFileName : "null");
if (result && result.resolvedModule) perfLogger?.logInfoEvent(`Module "${moduleName}" resolved to "${result.resolvedModule.resolvedFileName}"`);
perfLogger?.logStopResolveModule((result && result.resolvedModule) ? "" + result.resolvedModule.resolvedFileName : "null");

cache?.getOrCreateCacheForDirectory(containingDirectory, redirectedReference).set(moduleName, resolutionMode, result);
if (!isExternalModuleNameRelative(moduleName)) {
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1321,7 +1321,7 @@ export function createSourceFile(fileName: string, sourceText: string, languageV
performance.mark("beforeParse");
let result: SourceFile;

perfLogger.logStartParseSourceFile(fileName);
perfLogger?.logStartParseSourceFile(fileName);
const {
languageVersion,
setExternalModuleIndicator: overrideSetExternalModuleIndicator,
Expand All @@ -1337,7 +1337,7 @@ export function createSourceFile(fileName: string, sourceText: string, languageV
};
result = Parser.parseSourceFile(fileName, sourceText, languageVersion, /*syntaxCursor*/ undefined, setParentNodes, scriptKind, setIndicator);
}
perfLogger.logStopParseSourceFile();
perfLogger?.logStopParseSourceFile();

performance.mark("afterParse");
performance.measure("Parse", "beforeParse", "afterParse");
Expand Down
26 changes: 1 addition & 25 deletions src/compiler/perfLogger.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { noop } from "./_namespaces/ts";

/** @internal */
export interface PerfLogger {
logEvent(msg: string): void;
Expand All @@ -24,28 +22,6 @@ export interface PerfLogger {
logStopScheduledOperation(): void;
}

const nullLogger: PerfLogger = {
logEvent: noop,
logErrEvent: noop,
logPerfEvent: noop,
logInfoEvent: noop,
logStartCommand: noop,
logStopCommand: noop,
logStartUpdateProgram: noop,
logStopUpdateProgram: noop,
logStartUpdateGraph: noop,
logStopUpdateGraph: noop,
logStartResolveModule: noop,
logStopResolveModule: noop,
logStartParseSourceFile: noop,
logStopParseSourceFile: noop,
logStartReadFile: noop,
logStopReadFile: noop,
logStartBindFile: noop,
logStopBindFile: noop,
logStartScheduledOperation: noop,
logStopScheduledOperation: noop,
};

// Load optional module to enable Event Tracing for Windows
// See https://github.com/microsoft/typescript-etw for more information
Expand All @@ -66,4 +42,4 @@ catch (e) {
*
* @internal
*/
export const perfLogger: PerfLogger = etwModule?.logEvent ? etwModule : nullLogger;
export const perfLogger: PerfLogger | undefined = etwModule?.logEvent ? etwModule : undefined;
8 changes: 4 additions & 4 deletions src/compiler/sys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1814,14 +1814,14 @@ export let sys: System = (() => {
}

function readFile(fileName: string, _encoding?: string): string | undefined {
perfLogger.logStartReadFile(fileName);
perfLogger?.logStartReadFile(fileName);
const file = readFileWorker(fileName, _encoding);
perfLogger.logStopReadFile();
perfLogger?.logStopReadFile();
return file;
}

function writeFile(fileName: string, data: string, writeByteOrderMark?: boolean): void {
perfLogger.logEvent("WriteFile: " + fileName);
perfLogger?.logEvent("WriteFile: " + fileName);
// If a BOM is required, emit one
if (writeByteOrderMark) {
data = byteOrderMarkIndicator + data;
Expand All @@ -1841,7 +1841,7 @@ export let sys: System = (() => {
}

function getAccessibleFileSystemEntries(path: string): FileSystemEntries {
perfLogger.logEvent("ReadDir: " + (path || "."));
perfLogger?.logEvent("ReadDir: " + (path || "."));
try {
const entries = _fs.readdirSync(path || ".", { withFileTypes: true });
const files: string[] = [];
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/watchPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -843,19 +843,19 @@ export function createWatchProgram<T extends BuilderProgram>(host: WatchCompiler
function updateProgram() {
switch (reloadLevel) {
case ConfigFileProgramReloadLevel.Partial:
perfLogger.logStartUpdateProgram("PartialConfigReload");
perfLogger?.logStartUpdateProgram("PartialConfigReload");
reloadFileNamesFromConfigFile();
break;
case ConfigFileProgramReloadLevel.Full:
perfLogger.logStartUpdateProgram("FullConfigReload");
perfLogger?.logStartUpdateProgram("FullConfigReload");
reloadConfigFile();
break;
default:
perfLogger.logStartUpdateProgram("SynchronizeProgram");
perfLogger?.logStartUpdateProgram("SynchronizeProgram");
synchronizeProgram();
break;
}
perfLogger.logStopUpdateProgram("Done");
perfLogger?.logStopUpdateProgram("Done");
return getCurrentBuilderProgram();
}

Expand Down
4 changes: 2 additions & 2 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
*/
updateGraph(): boolean {
tracing?.push(tracing.Phase.Session, "updateGraph", { name: this.projectName, kind: ProjectKind[this.projectKind] });
perfLogger.logStartUpdateGraph();
perfLogger?.logStartUpdateGraph();
this.resolutionCache.startRecordingFilesWithChangedResolutions();

const hasNewProgram = this.updateGraphWorker();
Expand Down Expand Up @@ -1305,7 +1305,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
// Preload auto import provider so it's not created during completions request
this.getPackageJsonAutoImportProvider();
}
perfLogger.logStopUpdateGraph();
perfLogger?.logStopUpdateGraph();
tracing?.pop();
return !hasNewProgram;
}
Expand Down
10 changes: 5 additions & 5 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ export class Session<TMessage = string> implements EventSender {

protected writeMessage(msg: protocol.Message) {
const msgText = formatMessage(msg, this.logger, this.byteLength, this.host.newLine);
perfLogger.logEvent(`Response message size: ${msgText.length}`);
perfLogger?.logEvent(`Response message size: ${msgText.length}`);
this.host.write(msgText);
}

Expand Down Expand Up @@ -3540,7 +3540,7 @@ export class Session<TMessage = string> implements EventSender {
relevantFile = request.arguments && (request as protocol.FileRequest).arguments.file ? (request as protocol.FileRequest).arguments : undefined;

tracing?.instant(tracing.Phase.Session, "request", { seq: request.seq, command: request.command });
perfLogger.logStartCommand("" + request.command, this.toStringMessage(message).substring(0, 100));
perfLogger?.logStartCommand("" + request.command, this.toStringMessage(message).substring(0, 100));

tracing?.push(tracing.Phase.Session, "executeCommand", { seq: request.seq, command: request.command }, /*separateBeginAndEnd*/ true);
const { response, responseRequired } = this.executeCommand(request);
Expand All @@ -3557,7 +3557,7 @@ export class Session<TMessage = string> implements EventSender {
}

// Note: Log before writing the response, else the editor can complete its activity before the server does
perfLogger.logStopCommand("" + request.command, "Success");
perfLogger?.logStopCommand("" + request.command, "Success");
tracing?.instant(tracing.Phase.Session, "response", { seq: request.seq, command: request.command, success: !!response });
if (response) {
this.doOutput(response, request.command, request.seq, /*success*/ true);
Expand All @@ -3572,14 +3572,14 @@ export class Session<TMessage = string> implements EventSender {

if (err instanceof OperationCanceledException) {
// Handle cancellation exceptions
perfLogger.logStopCommand("" + (request && request.command), "Canceled: " + err);
perfLogger?.logStopCommand("" + (request && request.command), "Canceled: " + err);
tracing?.instant(tracing.Phase.Session, "commandCanceled", { seq: request?.seq, command: request?.command });
this.doOutput({ canceled: true }, request!.command, request!.seq, /*success*/ true);
return;
}

this.logErrorWorker(err, this.toStringMessage(message), relevantFile);
perfLogger.logStopCommand("" + (request && request.command), "Error: " + err);
perfLogger?.logStopCommand("" + (request && request.command), "Error: " + err);
tracing?.instant(tracing.Phase.Session, "commandError", { seq: request?.seq, command: request?.command, message: (err as Error).message });

this.doOutput(
Expand Down
8 changes: 4 additions & 4 deletions src/server/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ export class ThrottledOperations {
}

private static run(self: ThrottledOperations, operationId: string, cb: () => void) {
perfLogger.logStartScheduledOperation(operationId);
perfLogger?.logStartScheduledOperation(operationId);
self.pendingTimeouts.delete(operationId);
if (self.logger) {
self.logger.info(`Running: ${operationId}`);
}
cb();
perfLogger.logStopScheduledOperation();
perfLogger?.logStopScheduledOperation();
}
}

Expand All @@ -75,7 +75,7 @@ export class GcTimer {
private static run(self: GcTimer) {
self.timerId = undefined;

perfLogger.logStartScheduledOperation("GC collect");
perfLogger?.logStartScheduledOperation("GC collect");
const log = self.logger.hasLevel(LogLevel.requestTime);
const before = log && self.host.getMemoryUsage!(); // TODO: GH#18217

Expand All @@ -84,7 +84,7 @@ export class GcTimer {
const after = self.host.getMemoryUsage!(); // TODO: GH#18217
self.logger.perftrc(`GC::before ${before}, after ${after}`);
}
perfLogger.logStopScheduledOperation();
perfLogger?.logStopScheduledOperation();
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/tsserver/nodeServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,13 @@ export function initializeNodeSystem(): StartInput {
msg(s: string, type: Msg = Msg.Err) {
switch (type) {
case Msg.Info:
perfLogger.logInfoEvent(s);
perfLogger?.logInfoEvent(s);
break;
case Msg.Perf:
perfLogger.logPerfEvent(s);
perfLogger?.logPerfEvent(s);
break;
default: // Msg.Err
perfLogger.logErrEvent(s);
perfLogger?.logErrEvent(s);
break;
}

Expand Down