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)

* .
This commit is contained in:
Benjamin Pasero
2026-02-12 14:58:23 +01:00
committed by GitHub
parent 6924a14212
commit 855722ab9c
10 changed files with 163 additions and 70 deletions

View File

@@ -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
}
});
});
}
};

View File

@@ -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',

View File

@@ -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<number>((resolvePort, rejectPort) => {
server = http.createServer((req, res) => {
if (req.url === '/noreply') {

View File

@@ -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';

View File

@@ -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<void> {
const { createServer } = await import('http'); // Lazy due to https://github.com/nodejs/node/issues/59686
const deferredPromise = new DeferredPromise<void>();
this._server = http.createServer((req, res) => {
this._server = createServer((req, res) => {
this._handleRequest(req, res);
});

View File

@@ -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<string> {
async function sendToPipe(args: PipeCommand, verbose: boolean): Promise<string> {
const http = await import('http');
if (verbose) {
console.log(JSON.stringify(args, null, ' '));
}

View File

@@ -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';

View File

@@ -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';

View File

@@ -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<string> {
private async setup(): Promise<void> {
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);

View File

@@ -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<http.Server>;
private readonly _resultPromise: Promise<IOAuthResult>;
private _state = randomBytes(16).toString('base64');
@@ -56,45 +56,49 @@ export class LoopbackAuthServer implements ILoopbackServer {
const deferredPromise = new DeferredPromise<IOAuthResult>();
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<void> {
async start(): Promise<void> {
const server = await this._server;
const deferredPromise = new DeferredPromise<void>();
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<void> {
async stop(): Promise<void> {
const deferredPromise = new DeferredPromise<void>();
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 {