diff --git a/CHANGELOG.md b/CHANGELOG.md index 134a80ab4..9347120df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he ## Nightly (only) - fix: improve main-thread performance of source map rename ([vscode#210518](https://github.com/microsoft/vscode/issues/210518)) +- fix: improve protocol handling performance in all cases ([#2001](https://github.com/microsoft/vscode-js-debug/issues/2001)) ## v1.89 (April 2024) diff --git a/package-lock.json b/package-lock.json index a7af957d9..e53c6406b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38,7 +38,6 @@ "reflect-metadata": "^0.2.1", "signale": "^1.4.0", "source-map-support": "^0.5.21", - "split2": "^4.2.0", "to-absolute-glob": "^3.0.0", "vscode-tas-client": "^0.1.84", "ws": "^8.16.0" @@ -71,7 +70,6 @@ "@types/prettier": "^2.7.3", "@types/signale": "^1.4.7", "@types/sinon": "^17.0.3", - "@types/split2": "^4.2.3", "@types/stream-buffers": "^3.0.7", "@types/tmp": "^0.2.6", "@types/to-absolute-glob": "^2.0.3", @@ -2188,15 +2186,6 @@ "integrity": "sha512-9GcLXF0/v3t80caGs5p2rRfkB+a8VBGLJZVih6CNFkx8IZ994wiKKLSRs9nuFwk1HevWs/1mnUmkApGrSGsShA==", "dev": true }, - "node_modules/@types/split2": { - "version": "4.2.3", - "resolved": "https://registry.npmjs.org/@types/split2/-/split2-4.2.3.tgz", - "integrity": "sha512-59OXIlfUsi2k++H6CHgUQKEb2HKRokUA39HY1i1dS8/AIcqVjtAAFdf8u+HxTWK/4FUHMJQlKSZ4I6irCBJ1Zw==", - "dev": true, - "dependencies": { - "@types/node": "*" - } - }, "node_modules/@types/stream-buffers": { "version": "3.0.7", "resolved": "https://registry.npmjs.org/@types/stream-buffers/-/stream-buffers-3.0.7.tgz", @@ -13515,14 +13504,6 @@ "node": ">=0.10.0" } }, - "node_modules/split2": { - "version": "4.2.0", - "resolved": "https://registry.npmjs.org/split2/-/split2-4.2.0.tgz", - "integrity": "sha512-UcjcJOWknrNkF6PLX83qcHM6KHgVKNkV62Y8a5uYDVv9ydGQVwAHMKqHdJje1VTWpljG0WYpCDhrCdAOYH4TWg==", - "engines": { - "node": ">= 10.x" - } - }, "node_modules/sprintf-js": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz", @@ -17408,15 +17389,6 @@ "integrity": "sha512-9GcLXF0/v3t80caGs5p2rRfkB+a8VBGLJZVih6CNFkx8IZ994wiKKLSRs9nuFwk1HevWs/1mnUmkApGrSGsShA==", "dev": true }, - "@types/split2": { - "version": "4.2.3", - "resolved": "https://registry.npmjs.org/@types/split2/-/split2-4.2.3.tgz", - "integrity": "sha512-59OXIlfUsi2k++H6CHgUQKEb2HKRokUA39HY1i1dS8/AIcqVjtAAFdf8u+HxTWK/4FUHMJQlKSZ4I6irCBJ1Zw==", - "dev": true, - "requires": { - "@types/node": "*" - } - }, "@types/stream-buffers": { "version": "3.0.7", "resolved": "https://registry.npmjs.org/@types/stream-buffers/-/stream-buffers-3.0.7.tgz", @@ -26168,11 +26140,6 @@ "extend-shallow": "^3.0.0" } }, - "split2": { - "version": "4.2.0", - "resolved": "https://registry.npmjs.org/split2/-/split2-4.2.0.tgz", - "integrity": "sha512-UcjcJOWknrNkF6PLX83qcHM6KHgVKNkV62Y8a5uYDVv9ydGQVwAHMKqHdJje1VTWpljG0WYpCDhrCdAOYH4TWg==" - }, "sprintf-js": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.0.3.tgz", diff --git a/package.json b/package.json index 7ae05f55a..567a8154c 100644 --- a/package.json +++ b/package.json @@ -78,7 +78,6 @@ "reflect-metadata": "^0.2.1", "signale": "^1.4.0", "source-map-support": "^0.5.21", - "split2": "^4.2.0", "to-absolute-glob": "^3.0.0", "vscode-tas-client": "^0.1.84", "ws": "^8.16.0" @@ -118,7 +117,6 @@ "@types/prettier": "^2.7.3", "@types/signale": "^1.4.7", "@types/sinon": "^17.0.3", - "@types/split2": "^4.2.3", "@types/stream-buffers": "^3.0.7", "@types/tmp": "^0.2.6", "@types/to-absolute-glob": "^2.0.3", diff --git a/src/cdp/rawPipeTransport.ts b/src/cdp/rawPipeTransport.ts index 29963feef..36932aaa3 100644 --- a/src/cdp/rawPipeTransport.ts +++ b/src/cdp/rawPipeTransport.ts @@ -2,12 +2,12 @@ * Copyright (C) Microsoft Corporation. All rights reserved. *--------------------------------------------------------*/ -import split from 'split2'; import { Duplex, Readable, Writable } from 'stream'; import { EventEmitter } from '../common/events'; import { HrTime } from '../common/hrnow'; import { ILogger, LogTag } from '../common/logging'; import { once } from '../common/objUtils'; +import { StreamSplitter } from '../common/streamSplitter'; import { ITransport } from './transport'; export class RawPipeTransport implements ITransport { @@ -57,8 +57,8 @@ export class RawPipeTransport implements ITransport { this.streams = { read: read .on('error', error => this.logger.error(LogTag.Internal, 'pipeRead error', { error })) - .pipe(split('\0')) - .on('data', json => this.messageEmitter.fire([json, new HrTime()])) + .pipe(new StreamSplitter(0)) + .on('data', (json: Buffer) => this.messageEmitter.fire([json.toString(), new HrTime()])) .on('end', this.onceEnded), write: pipeWrite.on('end', this.onceEnded).on('error', this.onWriteError), }; diff --git a/src/common/streamSplitter.test.ts b/src/common/streamSplitter.test.ts new file mode 100644 index 000000000..a43f80869 --- /dev/null +++ b/src/common/streamSplitter.test.ts @@ -0,0 +1,29 @@ +/*--------------------------------------------------------- + * Copyright (C) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------*/ + +import * as assert from 'assert'; +import { Writable } from 'stream'; +import { StreamSplitter } from './streamSplitter'; + +describe('StreamSplitter', () => { + it('should split a stream', done => { + const chunks: string[] = []; + const splitter = new StreamSplitter('\n'); + const writable = new Writable({ + write(chunk, _encoding, callback) { + chunks.push(chunk.toString()); + callback(); + }, + }); + + splitter.pipe(writable); + splitter.write('hello\nwor'); + splitter.write('ld\n'); + splitter.write('foo\nbar\nz'); + splitter.end(() => { + assert.deepStrictEqual(chunks, ['hello', 'world', 'foo', 'bar', 'z']); + done(); + }); + }); +}); diff --git a/src/common/streamSplitter.ts b/src/common/streamSplitter.ts new file mode 100644 index 000000000..15805b8d2 --- /dev/null +++ b/src/common/streamSplitter.ts @@ -0,0 +1,70 @@ +/*--------------------------------------------------------- + * Copyright (C) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------*/ + +// Based on VS Code's src/vs/base/node/nodeStreams.ts + +import { Transform } from 'stream'; + +/** + * A Transform stream that splits the input on the "splitter" substring. + * The resulting chunks will contain (and trail with) the splitter match. + * The last chunk when the stream ends will be emitted even if a splitter + * is not encountered. + */ +export class StreamSplitter extends Transform { + private prefix: Buffer[] = []; + private readonly splitter: number; + + /** Suffix added after each split chunk. */ + protected splitSuffix = Buffer.alloc(0); + + constructor(splitter: string | number | Buffer) { + super(); + if (typeof splitter === 'string' && splitter.length === 1) { + this.splitter = splitter.charCodeAt(0); + } else if (typeof splitter === 'number') { + this.splitter = splitter; + } else { + throw new Error('not implemented here'); + } + } + + override _transform( + chunk: Buffer, + _encoding: string, + callback: (error?: Error | null, data?: unknown) => void, + ): void { + let offset = 0; + while (offset < chunk.length) { + const index = chunk.indexOf(this.splitter, offset); + if (index === -1) { + break; + } + + const thisChunk = chunk.subarray(offset, index); + const toEmit = + this.prefix.length || this.splitSuffix.length + ? Buffer.concat([...this.prefix, thisChunk, this.splitSuffix]) + : thisChunk; + + this.push(toEmit); + this.prefix.length = 0; + offset = index + 1; + } + + if (offset < chunk.length) { + this.prefix.push(chunk.subarray(offset)); + } + + callback(); + } + + override _flush(callback: (error?: Error | null, data?: unknown) => void): void { + for (const buf of this.prefix) { + this.push(buf); + } + + callback(); + } +} diff --git a/src/targets/node/subprocessProgramLauncher.ts b/src/targets/node/subprocessProgramLauncher.ts index 3a3122a20..fe27a4fee 100644 --- a/src/targets/node/subprocessProgramLauncher.ts +++ b/src/targets/node/subprocessProgramLauncher.ts @@ -4,15 +4,14 @@ import { ChildProcessWithoutNullStreams, spawn } from 'child_process'; import { inject, injectable } from 'inversify'; -import split from 'split2'; -import { Transform } from 'stream'; import { EnvironmentVars } from '../../common/environmentVars'; import { ILogger } from '../../common/logging'; +import { StreamSplitter } from '../../common/streamSplitter'; import * as urlUtils from '../../common/urlUtils'; import { INodeLaunchConfiguration, OutputSource } from '../../configuration'; import Dap from '../../dap/api'; import { ILaunchContext } from '../targets'; -import { getNodeLaunchArgs, IProgramLauncher } from './processLauncher'; +import { IProgramLauncher, getNodeLaunchArgs } from './processLauncher'; import { SubprocessProgram } from './program'; /** @@ -64,12 +63,12 @@ export class SubprocessProgramLauncher implements IProgramLauncher { */ private captureStdio(dap: Dap.Api, child: ChildProcessWithoutNullStreams) { child.stdout - .pipe(EtxSplitter.stream()) - .on('data', output => dap.output({ category: 'stdout', output })) + .pipe(new EtxSplitter()) + .on('data', output => dap.output({ category: 'stdout', output: output.toString() })) .resume(); child.stderr - .pipe(EtxSplitter.stream()) - .on('data', output => dap.output({ category: 'stderr', output })) + .pipe(new EtxSplitter()) + .on('data', output => dap.output({ category: 'stderr', output: output.toString() })) .resume(); } @@ -158,7 +157,7 @@ const formatArguments = (executable: string, args: ReadonlyArray, cwd: s }; const enum Char { - ETX = '\u0003', + ETX = 3, } /** @@ -168,22 +167,28 @@ const enum Char { * finds any, it will switch from splitting the stream on newlines to * splitting on ETX characters. */ -export class EtxSplitter { +export class EtxSplitter extends StreamSplitter { private etxSpotted = false; - public static stream(): Transform { - return split(new EtxSplitter()); + constructor() { + super(Char.ETX); + this.splitSuffix = Buffer.from('\n'); } - [Symbol.split](str: string) { - this.etxSpotted ||= str.includes(Char.ETX); - if (!this.etxSpotted) { - return [str, '']; + override _transform( + chunk: Buffer, + _encoding: string, + callback: (error?: Error | null | undefined, data?: unknown) => void, + ): void { + if (!this.etxSpotted && chunk.includes(Char.ETX)) { + this.etxSpotted = true; } - const split = str.split(Char.ETX); + if (!this.etxSpotted) { + this.push(chunk); + return callback(); + } - // restore or add new lines between each record for proper debug console display - return split.length > 1 ? split.map((s, i) => (i < split.length - 1 ? `${s}\n` : s)) : split; + return super._transform(chunk, _encoding, callback); } } diff --git a/src/test/extension/pickAttach.test.ts b/src/test/extension/pickAttach.test.ts index 513721f75..c04bd3fb1 100644 --- a/src/test/extension/pickAttach.test.ts +++ b/src/test/extension/pickAttach.test.ts @@ -8,12 +8,12 @@ import { promises as fsPromises } from 'fs'; import { tmpdir } from 'os'; import * as path from 'path'; import { SinonSandbox, createSandbox } from 'sinon'; -import split from 'split2'; import * as vscode from 'vscode'; import { Commands, DebugType } from '../../common/contributionUtils'; import { findOpenPort } from '../../common/findOpenPort'; import { LocalFsUtils } from '../../common/fsUtils'; import { delay } from '../../common/promiseUtil'; +import { StreamSplitter } from '../../common/streamSplitter'; import { nodeAttachConfigDefaults } from '../../configuration'; import { resolveProcessId } from '../../ui/processPicker'; import { createFileTree } from '../createFileTree'; @@ -89,8 +89,11 @@ describe('pick and attach', () => { child = spawn('node', ['--inspect-brk', `--inspect-port=${port}`], { stdio: 'pipe' }); child.on('error', console.error); child - .stderr!.pipe(split()) - .on('data', (line: string) => (attached = attached || line.includes('Debugger attached'))); + .stderr!.pipe(new StreamSplitter('\n')) + .on( + 'data', + (line: string) => (attached = attached || line.toString().includes('Debugger attached')), + ); }); it('end to end', async function () { diff --git a/src/test/node/node-runtime.test.ts b/src/test/node/node-runtime.test.ts index 2030139b9..830ac6d36 100644 --- a/src/test/node/node-runtime.test.ts +++ b/src/test/node/node-runtime.test.ts @@ -7,13 +7,17 @@ import { ChildProcess, spawn } from 'child_process'; import { promises as fsPromises } from 'fs'; import { dirname, join } from 'path'; import { stub } from 'sinon'; -import split from 'split2'; import { EnvironmentVars } from '../../common/environmentVars'; import { findOpenPort } from '../../common/findOpenPort'; import { once } from '../../common/objUtils'; import { findInPath } from '../../common/pathUtils'; import { delay } from '../../common/promiseUtil'; -import { INodeLaunchConfiguration, nodeLaunchConfigDefaults } from '../../configuration'; +import { StreamSplitter } from '../../common/streamSplitter'; +import { + INodeLaunchConfiguration, + OutputSource, + nodeLaunchConfigDefaults, +} from '../../configuration'; import Dap from '../../dap/api'; import { TerminalProgramLauncher } from '../../targets/node/terminalProgramLauncher'; import { createFileTree } from '../createFileTree'; @@ -441,7 +445,7 @@ describe('node runtime', () => { stdio: 'pipe', }); const lines: string[] = []; - child.stderr?.pipe(split()).on('data', line => lines.push(line)); + child.stderr?.pipe(new StreamSplitter('\n')).on('data', line => lines.push(line.toString())); const handle = await r.attachNode(0, { port, restart: true }); await handle.dap.once('stopped'); @@ -712,4 +716,46 @@ describe('node runtime', () => { handle.assertLog({ substring: true }); }); }); + + describe('etx', () => { + itIntegrates('stdio without etx', async ({ r }) => { + await r.initialize; + + createFileTree(testFixturesDir, { + 'test.js': ['process.stdout.write("hello world!");', 'debugger;'], + }); + const handle = await r.runScript('test.js', { outputCapture: OutputSource.Stdio }); + handle.load(); + let logs = ''; + r.rootDap().on('output', p => (logs += p.output)); + await handle.dap.once('stopped'); + expect(logs).to.deep.equal('hello world!'); + }); + + itIntegrates('stdio with etx', async ({ r }) => { + await r.initialize; + + createFileTree(testFixturesDir, { + 'test.js': [ + 'process.stdout.write("etx\u0003start");', + 'process.stdout.write("well defined\u0003");', + 'process.stdout.write("chunks\u0003\u0003now!\u0003");', + ], + }); + const handle = await r.runScript('test.js', { outputCapture: OutputSource.Stdio }); + handle.load(); + const logs: string[] = []; + r.rootDap().on('output', p => logs.push(p.output)); + await handle.dap.once('terminated'); + + expect(logs).to.deep.equal([ + 'etx\n', + 'startwell defined\n', + 'chunks\n', + '\n', + 'now!\n', + 'Waiting for the debugger to disconnect...\n', + ]); + }); + }); }); diff --git a/src/ui/processTree/baseProcessTree.ts b/src/ui/processTree/baseProcessTree.ts index ae23cb0c0..7460f1a01 100644 --- a/src/ui/processTree/baseProcessTree.ts +++ b/src/ui/processTree/baseProcessTree.ts @@ -3,7 +3,7 @@ *--------------------------------------------------------*/ import { ChildProcessWithoutNullStreams, spawn as defaultSpawn } from 'child_process'; -import split2 from 'split2'; +import { StreamSplitter } from '../../common/streamSplitter'; import { IProcess, IProcessTree } from './processTree'; /** @@ -27,8 +27,8 @@ export abstract class BaseProcessTree implements IProcessTree { proc.on('error', reject); proc.stderr.on('error', data => reject(`Error finding processes: ${data.toString()}`)); - proc.stdout.pipe(split2(/\r?\n/)).on('data', line => { - const process = parser(line); + proc.stdout.pipe(new StreamSplitter('\n')).on('data', line => { + const process = parser(line.toString().trim()); if (process) { value = onEntry(process, value); }