From 855722ab9c2f735d08ab90603903aead23a31f19 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 12 Feb 2026 14:58:23 +0100 Subject: [PATCH] Startup perf regression due to top level import of `http` (fix #294857) (#294894) * Startup perf regression due to top level import of http (fix #294857) * Startup perf regression due to top level import of `http` (fix #294857) * . --- .eslint-plugin-local/code-no-http-import.ts | 78 ++++++++++++++ eslint.config.js | 1 + .../test/electron-main/request.test.ts | 3 +- .../electron-main/extensionHostDebugIpc.ts | 2 +- src/vs/platform/mcp/node/mcpGatewayService.ts | 5 +- src/vs/server/node/server.cli.ts | 5 +- src/vs/server/node/serverConnectionToken.ts | 2 +- src/vs/server/node/webClientServer.ts | 2 +- src/vs/workbench/api/node/extHostCLIServer.ts | 35 +++--- src/vs/workbench/api/node/loopbackServer.ts | 100 ++++++++++-------- 10 files changed, 163 insertions(+), 70 deletions(-) create mode 100644 .eslint-plugin-local/code-no-http-import.ts diff --git a/.eslint-plugin-local/code-no-http-import.ts b/.eslint-plugin-local/code-no-http-import.ts new file mode 100644 index 00000000000..d968065a34c --- /dev/null +++ b/.eslint-plugin-local/code-no-http-import.ts @@ -0,0 +1,78 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { TSESTree } from '@typescript-eslint/typescript-estree'; +import * as eslint from 'eslint'; +import { normalize } from 'path'; +import minimatch from 'minimatch'; +import { createImportRuleListener } from './utils.ts'; + +const restrictedModules = new Set(['http', 'https']); + +const REPO_ROOT = normalize(`${import.meta.dirname}/..`); + +export default new class implements eslint.Rule.RuleModule { + + readonly meta: eslint.Rule.RuleMetaData = { + messages: { + notAllowed: 'Importing \'{{module}}\' is only allowed as a type import (`import type ...`) to prevent startup performance regressions as these modules are slow to load. Use dynamic `import(\'{{module}}\')` for runtime access.' + }, + schema: { + type: 'array', + items: { + type: 'object', + properties: { + target: { + type: 'string', + description: 'A glob pattern for files to check' + } + }, + additionalProperties: false, + required: ['target'] + } + } + }; + + create(context: eslint.Rule.RuleContext): eslint.Rule.RuleListener { + const targets = (context.options as { target: string }[]).map(o => o.target); + if (targets.length > 0) { + const relativeFilename = normalize(context.getFilename()).substring(REPO_ROOT.length + 1).replace(/\\/g, '/'); + const matched = targets.some(pattern => minimatch(relativeFilename, pattern)); + if (!matched) { + return {}; // file is not covered by any target pattern + } + } + + return createImportRuleListener((node, path) => { + if (!restrictedModules.has(path)) { + return; + } + + const parent = node.parent; + if (!parent) { + return; + } + + // Allow: import type { ... } from 'http' + // Allow: import type * as http from 'http' + if (parent.type === TSESTree.AST_NODE_TYPES.ImportDeclaration && parent.importKind === 'type') { + return; + } + + // Allow: export type { ... } from 'http' + if ('exportKind' in parent && parent.exportKind === 'type') { + return; + } + + context.report({ + loc: parent.loc, + messageId: 'notAllowed', + data: { + module: path + } + }); + }); + } +}; diff --git a/eslint.config.js b/eslint.config.js index f60ff793db5..dfb489e0f0b 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -92,6 +92,7 @@ export default tseslint.config( 'local/code-no-localized-model-description': 'warn', 'local/code-policy-localization-key-match': 'warn', 'local/code-no-localization-template-literals': 'error', + 'local/code-no-http-import': ['warn', { target: 'src/vs/**' }], 'local/code-no-deep-import-of-internal': ['error', { '.*Internal': true, 'searchExtTypesInternal': false }], 'local/code-layering': [ 'warn', diff --git a/src/vs/base/parts/request/test/electron-main/request.test.ts b/src/vs/base/parts/request/test/electron-main/request.test.ts index e653570c5fc..895b5dc6899 100644 --- a/src/vs/base/parts/request/test/electron-main/request.test.ts +++ b/src/vs/base/parts/request/test/electron-main/request.test.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as http from 'http'; +import type * as http from 'http'; import { AddressInfo } from 'net'; import assert from 'assert'; import { CancellationToken, CancellationTokenSource } from '../../../../common/cancellation.js'; @@ -19,6 +19,7 @@ suite('Request', () => { let server: http.Server; setup(async () => { + const http = await import('http'); port = await new Promise((resolvePort, rejectPort) => { server = http.createServer((req, res) => { if (req.url === '/noreply') { diff --git a/src/vs/platform/debug/electron-main/extensionHostDebugIpc.ts b/src/vs/platform/debug/electron-main/extensionHostDebugIpc.ts index 0a12741a00e..40f86feaa88 100644 --- a/src/vs/platform/debug/electron-main/extensionHostDebugIpc.ts +++ b/src/vs/platform/debug/electron-main/extensionHostDebugIpc.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { BrowserWindow } from 'electron'; -import { Server } from 'http'; +import type { Server } from 'http'; import { Socket } from 'net'; import { VSBuffer } from '../../../base/common/buffer.js'; import { DisposableStore, toDisposable } from '../../../base/common/lifecycle.js'; diff --git a/src/vs/platform/mcp/node/mcpGatewayService.ts b/src/vs/platform/mcp/node/mcpGatewayService.ts index e4ee1fe42bb..f82cac22a63 100644 --- a/src/vs/platform/mcp/node/mcpGatewayService.ts +++ b/src/vs/platform/mcp/node/mcpGatewayService.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as http from 'http'; +import type * as http from 'http'; import { DeferredPromise } from '../../../base/common/async.js'; import { Disposable } from '../../../base/common/lifecycle.js'; import { URI } from '../../../base/common/uri.js'; @@ -124,9 +124,10 @@ export class McpGatewayService extends Disposable implements IMcpGatewayService } private async _startServer(): Promise { + const { createServer } = await import('http'); // Lazy due to https://github.com/nodejs/node/issues/59686 const deferredPromise = new DeferredPromise(); - this._server = http.createServer((req, res) => { + this._server = createServer((req, res) => { this._handleRequest(req, res); }); diff --git a/src/vs/server/node/server.cli.ts b/src/vs/server/node/server.cli.ts index 6a0eacffc56..58ff362b98a 100644 --- a/src/vs/server/node/server.cli.ts +++ b/src/vs/server/node/server.cli.ts @@ -6,7 +6,7 @@ import * as fs from 'fs'; import * as url from 'url'; import * as cp from 'child_process'; -import * as http from 'http'; +import type * as http from 'http'; import { cwd } from '../../base/common/process.js'; import { dirname, extname, resolve, join } from '../../base/common/path.js'; import { parseArgs, buildHelpMessage, buildVersionMessage, OPTIONS, OptionDescriptions, ErrorReporter } from '../../platform/environment/node/argv.js'; @@ -412,7 +412,8 @@ async function openInBrowser(args: string[], verbose: boolean) { } } -function sendToPipe(args: PipeCommand, verbose: boolean): Promise { +async function sendToPipe(args: PipeCommand, verbose: boolean): Promise { + const http = await import('http'); if (verbose) { console.log(JSON.stringify(args, null, ' ')); } diff --git a/src/vs/server/node/serverConnectionToken.ts b/src/vs/server/node/serverConnectionToken.ts index 11089eea3c3..978d1b5eb41 100644 --- a/src/vs/server/node/serverConnectionToken.ts +++ b/src/vs/server/node/serverConnectionToken.ts @@ -5,7 +5,7 @@ import * as cookie from 'cookie'; import * as fs from 'fs'; -import * as http from 'http'; +import type * as http from 'http'; import * as url from 'url'; import * as path from '../../base/common/path.js'; import { generateUuid } from '../../base/common/uuid.js'; diff --git a/src/vs/server/node/webClientServer.ts b/src/vs/server/node/webClientServer.ts index 13882a9b44b..7881ad0393d 100644 --- a/src/vs/server/node/webClientServer.ts +++ b/src/vs/server/node/webClientServer.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { createReadStream, promises } from 'fs'; -import * as http from 'http'; +import type * as http from 'http'; import * as url from 'url'; import * as cookie from 'cookie'; import * as crypto from 'crypto'; diff --git a/src/vs/workbench/api/node/extHostCLIServer.ts b/src/vs/workbench/api/node/extHostCLIServer.ts index 0353ab3a9de..70b04f8fdde 100644 --- a/src/vs/workbench/api/node/extHostCLIServer.ts +++ b/src/vs/workbench/api/node/extHostCLIServer.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { createRandomIPCHandle } from '../../../base/parts/ipc/node/ipc.net.js'; -import * as http from 'http'; +import type * as http from 'http'; import * as fs from 'fs'; import { IExtHostCommands } from '../common/extHostCommands.js'; import { IWindowOpenable, IOpenWindowOptions } from '../../../platform/window/common/window.js'; @@ -51,33 +51,37 @@ export interface ICommandsExecuter { } export class CLIServerBase { - private readonly _server: http.Server; + private _server: http.Server | undefined = undefined; + private _disposed = false; constructor( private readonly _commands: ICommandsExecuter, private readonly logService: ILogService, private readonly _ipcHandlePath: string, ) { - this._server = http.createServer((req, res) => this.onRequest(req, res)); - this.setup().catch(err => { - logService.error(err); - return ''; - }); + this.setup(); } public get ipcHandlePath() { return this._ipcHandlePath; } - private async setup(): Promise { + private async setup(): Promise { try { - this._server.listen(this.ipcHandlePath); - this._server.on('error', err => this.logService.error(err)); - } catch (err) { - this.logService.error('Could not start open from terminal server.'); + const http = await import('http'); + if (this._disposed) { + return; + } + this._server = http.createServer((req, res) => this.onRequest(req, res)); + try { + this._server.listen(this.ipcHandlePath); + this._server.on('error', err => this.logService.error(err)); + } catch (err) { + this.logService.error('Could not start open from terminal server.'); + } + } catch (error) { + this.logService.error('Error setting up CLI server', error); } - - return this._ipcHandlePath; } private onRequest(req: http.IncomingMessage, res: http.ServerResponse): void { @@ -177,7 +181,8 @@ export class CLIServerBase { } dispose(): void { - this._server.close(); + this._disposed = true; + this._server?.close(); if (this._ipcHandlePath && process.platform !== 'win32' && fs.existsSync(this._ipcHandlePath)) { fs.unlinkSync(this._ipcHandlePath); diff --git a/src/vs/workbench/api/node/loopbackServer.ts b/src/vs/workbench/api/node/loopbackServer.ts index da3e4f5a549..1d32a486e87 100644 --- a/src/vs/workbench/api/node/loopbackServer.ts +++ b/src/vs/workbench/api/node/loopbackServer.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ import { randomBytes } from 'crypto'; -import * as http from 'http'; +import type * as http from 'http'; import { URL } from 'url'; import { DeferredPromise } from '../../../base/common/async.js'; import { DEFAULT_AUTH_FLOW_PORT } from '../../../base/common/oauth.js'; @@ -42,7 +42,7 @@ export interface ILoopbackServer { } export class LoopbackAuthServer implements ILoopbackServer { - private readonly _server: http.Server; + private readonly _server: Promise; private readonly _resultPromise: Promise; private _state = randomBytes(16).toString('base64'); @@ -56,45 +56,49 @@ export class LoopbackAuthServer implements ILoopbackServer { const deferredPromise = new DeferredPromise(); this._resultPromise = deferredPromise.p; - this._server = http.createServer((req, res) => { - const reqUrl = new URL(req.url!, `http://${req.headers.host}`); - switch (reqUrl.pathname) { - case '/': { - const code = reqUrl.searchParams.get('code') ?? undefined; - const state = reqUrl.searchParams.get('state') ?? undefined; - const error = reqUrl.searchParams.get('error') ?? undefined; - if (error) { - res.writeHead(302, { location: `/done?error=${reqUrl.searchParams.get('error_description') || error}` }); - res.end(); - deferredPromise.error(new Error(error)); - break; - } - if (!code || !state) { - res.writeHead(400); + this._server = (async () => { + const http = await import('http'); + + return http.createServer((req, res) => { + const reqUrl = new URL(req.url!, `http://${req.headers.host}`); + switch (reqUrl.pathname) { + case '/': { + const code = reqUrl.searchParams.get('code') ?? undefined; + const state = reqUrl.searchParams.get('state') ?? undefined; + const error = reqUrl.searchParams.get('error') ?? undefined; + if (error) { + res.writeHead(302, { location: `/done?error=${reqUrl.searchParams.get('error_description') || error}` }); + res.end(); + deferredPromise.error(new Error(error)); + break; + } + if (!code || !state) { + res.writeHead(400); + res.end(); + break; + } + if (this.state !== state) { + res.writeHead(302, { location: `/done?error=${encodeURIComponent('State does not match.')}` }); + res.end(); + deferredPromise.error(new Error('State does not match.')); + break; + } + deferredPromise.complete({ code, state }); + res.writeHead(302, { location: '/done' }); res.end(); break; } - if (this.state !== state) { - res.writeHead(302, { location: `/done?error=${encodeURIComponent('State does not match.')}` }); - res.end(); - deferredPromise.error(new Error('State does not match.')); + // Serve the static files + case '/done': + this._sendPage(res); + break; + default: + res.writeHead(404); + res.end(); break; - } - deferredPromise.complete({ code, state }); - res.writeHead(302, { location: '/done' }); - res.end(); - break; } - // Serve the static files - case '/done': - this._sendPage(res); - break; - default: - res.writeHead(404); - res.end(); - break; - } - }); + }); + })(); } get state(): string { return this._state; } @@ -114,16 +118,17 @@ export class LoopbackAuthServer implements ILoopbackServer { res.end(html); } - start(): Promise { + async start(): Promise { + const server = await this._server; const deferredPromise = new DeferredPromise(); - if (this._server.listening) { + if (server.listening) { throw new Error('Server is already started'); } const portTimeout = setTimeout(() => { deferredPromise.error(new Error('Timeout waiting for port')); }, 5000); - this._server.on('listening', () => { - const address = this._server.address(); + server.on('listening', () => { + const address = server.address(); if (typeof address === 'string') { this._port = parseInt(address); } else if (address instanceof Object) { @@ -135,31 +140,32 @@ export class LoopbackAuthServer implements ILoopbackServer { clearTimeout(portTimeout); deferredPromise.complete(); }); - this._server.on('error', err => { + server.on('error', err => { if ('code' in err && err.code === 'EADDRINUSE') { this._logger.error('Address in use, retrying with a different port...'); // Best effort to use a specific port, but fallback to a random one if it is in use - this._server.listen(0, '127.0.0.1'); + server.listen(0, '127.0.0.1'); return; } clearTimeout(portTimeout); deferredPromise.error(new Error(`Error listening to server: ${err}`)); }); - this._server.on('close', () => { + server.on('close', () => { deferredPromise.error(new Error('Closed')); }); // Best effort to use a specific port, but fallback to a random one if it is in use - this._server.listen(DEFAULT_AUTH_FLOW_PORT, '127.0.0.1'); + server.listen(DEFAULT_AUTH_FLOW_PORT, '127.0.0.1'); return deferredPromise.p; } - stop(): Promise { + async stop(): Promise { const deferredPromise = new DeferredPromise(); - if (!this._server.listening) { + const server = await this._server; + if (!server.listening) { deferredPromise.complete(); return deferredPromise.p; } - this._server.close((err) => { + server.close((err) => { if (err) { deferredPromise.error(err); } else {