From f3c945132c260e15a77698b8e2197ff06ae20e9e Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 16 Jul 2026 12:14:33 -0400 Subject: [PATCH] Walk the staging tree for .map files once per breakpoint pass, not once per breakpoint LocationManager.getStagingLocations ran a synchronous glob.sync('**/*.map') over the entire staging dir on every call, and getBreakpointWork calls it once per breakpoint. With N breakpoints that's N full-tree walks during breakpoint validation (on the launch path). This is the same anti-pattern that caused the findEntryPoint hang. - Add LocationManager.getStagingMapPaths(stagingDir) and let getStagingLocations accept a pre-computed list of map paths, falling back to walking the tree when omitted. - getBreakpointWork now walks once up front and passes the list to every getStagingLocations call, so the staging tree is walked a single time regardless of breakpoint count. Globbing fresh at the start of each validation pass (rather than caching across launches) keeps correctness when staging content changes between launches. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/managers/BreakpointManager.spec.ts | 22 ++++++++++++ src/managers/BreakpointManager.ts | 7 +++- src/managers/LocationManager.spec.ts | 49 ++++++++++++++++++++++++++ src/managers/LocationManager.ts | 23 +++++++++--- 4 files changed, 95 insertions(+), 6 deletions(-) diff --git a/src/managers/BreakpointManager.spec.ts b/src/managers/BreakpointManager.spec.ts index 826038ed..0ec95426 100644 --- a/src/managers/BreakpointManager.spec.ts +++ b/src/managers/BreakpointManager.spec.ts @@ -455,6 +455,28 @@ describe('BreakpointManager', () => { }); }); + it('walks the staging tree for .map files only once, regardless of breakpoint count', async () => { + //two files, multiple breakpoints each — the old code re-globbed the staging tree once per breakpoint + fsExtra.writeFileSync(`${rootDir}/source/main.brs`, `sub main()\n print 1\n print 2\n print 3\nend sub`); + fsExtra.writeFileSync(`${rootDir}/source/lib.brs`, `sub lib()\n print 1\n print 2\n print 3\nend sub`); + fsExtra.copyFileSync(`${rootDir}/source/main.brs`, `${stagingDir}/source/main.brs`); + fsExtra.copyFileSync(`${rootDir}/source/lib.brs`, `${stagingDir}/source/lib.brs`); + + bpManager.replaceBreakpoints(s`${rootDir}/source/main.brs`, [{ line: 2 }, { line: 3 }, { line: 4 }]); + bpManager.replaceBreakpoints(s`${rootDir}/source/lib.brs`, [{ line: 2 }, { line: 3 }, { line: 4 }]); + + const getStagingMapPaths = sinon.spy(locationManager, 'getStagingMapPaths'); + + await injectBreakpointsForProject(new Project({ + rootDir: rootDir, + outDir: outDir, + stagingDir: stagingDir + })); + + //6 breakpoints across 2 files, but the staging tree is walked exactly once + expect(getStagingMapPaths.callCount).to.equal(1); + }); + it('works with sourceDir1', async () => { //create file fsExtra.writeFileSync(`${sourceDir1}/source/main.brs`, `sub main()\n print 1\n print 2\nend sub`); diff --git a/src/managers/BreakpointManager.ts b/src/managers/BreakpointManager.ts index 9c00ffe8..44538bab 100644 --- a/src/managers/BreakpointManager.ts +++ b/src/managers/BreakpointManager.ts @@ -419,6 +419,10 @@ export class BreakpointManager { private async getBreakpointWork(project: Project, willInjectStop = false) { let result = {} as Record>; + //walk the staging tree for `.map` files ONCE up front, then reuse the list for every breakpoint. + //getStagingLocations would otherwise re-glob the entire staging dir on every single breakpoint. + const stagingMapPaths = this.locationManager.getStagingMapPaths(project.stagingDir); + //iterate over every file that contains breakpoints for (let [sourceFilePath, breakpoints] of this.breakpointsByFilePath) { for (let breakpoint of breakpoints) { @@ -438,7 +442,8 @@ export class BreakpointManager { project.rootDir ], project.stagingDir, - project.fileMappings + project.fileMappings, + stagingMapPaths ); for (let stagingLocation of stagingLocationsResult.locations) { diff --git a/src/managers/LocationManager.spec.ts b/src/managers/LocationManager.spec.ts index 3896c154..95241882 100644 --- a/src/managers/LocationManager.spec.ts +++ b/src/managers/LocationManager.spec.ts @@ -1,11 +1,14 @@ import { expect } from 'chai'; import * as fsExtra from 'fs-extra'; +import * as sinonActual from 'sinon'; import { SourceMapConsumer, SourceNode } from 'source-map'; import { standardizePath as s } from '../FileUtils'; import { LocationManager } from './LocationManager'; import { SourceMapManager } from './SourceMapManager'; import { forceDeleteDir } from '../testHelpers.spec'; +const sinon = sinonActual.createSandbox(); + let tempDir = s`${process.cwd()}/.tmp`; const rootDir = s`${tempDir}/rootDir`; const stagingDir = s`${tempDir}/stagingDir`; @@ -29,8 +32,54 @@ describe('LocationManager', () => { } }); afterEach(async () => { + sinon.restore(); await forceDeleteDir(tempDir); }); + + describe('getStagingMapPaths', () => { + it('returns all .map files in the staging dir', () => { + fsExtra.writeFileSync(s`${stagingDir}/source/main.brs.map`, '{}'); + fsExtra.writeFileSync(s`${stagingDir}/source/lib.brs.map`, '{}'); + //non-map files should be ignored + fsExtra.writeFileSync(s`${stagingDir}/source/main.brs`, ''); + + const result = locationManager.getStagingMapPaths(stagingDir).map(x => s`${x}`); + expect(result.sort()).to.eql([ + s`${stagingDir}/source/lib.brs.map`, + s`${stagingDir}/source/main.brs.map` + ].sort()); + }); + }); + + describe('getStagingLocations', () => { + it('uses the provided stagingMapPaths instead of walking the staging tree', async () => { + const getStagingMapPaths = sinon.spy(locationManager, 'getStagingMapPaths'); + const getGeneratedLocations = sinon.stub(sourceMapManager, 'getGeneratedLocations').returns(Promise.resolve([])); + + const providedMapPaths = [s`${stagingDir}/source/main.brs.map`]; + await locationManager.getStagingLocations( + s`${rootDir}/source/main.brs`, 1, 0, [], stagingDir, [], providedMapPaths + ); + + //it did NOT re-walk the staging tree + expect(getStagingMapPaths.called).to.be.false; + //it forwarded the provided map paths to the sourcemap lookup + expect(getGeneratedLocations.calledOnce).to.be.true; + expect(getGeneratedLocations.firstCall.args[0]).to.equal(providedMapPaths); + }); + + it('falls back to walking the staging tree when no stagingMapPaths are provided', async () => { + const getStagingMapPaths = sinon.spy(locationManager, 'getStagingMapPaths'); + sinon.stub(sourceMapManager, 'getGeneratedLocations').returns(Promise.resolve([])); + + await locationManager.getStagingLocations( + s`${rootDir}/source/main.brs`, 1, 0, [], stagingDir, [] + ); + + expect(getStagingMapPaths.calledOnce).to.be.true; + }); + }); + describe('getSourceLocation', () => { it('prevents infinite loop with circular dependency', async () => { diff --git a/src/managers/LocationManager.ts b/src/managers/LocationManager.ts index e0433898..73aeddb1 100644 --- a/src/managers/LocationManager.ts +++ b/src/managers/LocationManager.ts @@ -93,11 +93,26 @@ export class LocationManager { return undefined; } + /** + * Find every `.map` file in the staging folder. This walks the whole staging tree, so callers that + * resolve many source locations against the same staging dir (e.g. breakpoint validation) should + * call this once and pass the result into {@link getStagingLocations} rather than re-walking per call. + */ + public getStagingMapPaths(stagingDir: string): string[] { + return glob.sync('**/*.map', { + cwd: s`${stagingDir}`, + absolute: true + }); + } + /** * Given a source location, compute its locations in staging. You should call this for the main app (rootDir, rootDir+sourceDirs), * and also once for each component library. * There is a possibility of a single source location mapping to multiple staging locations (i.e. merging a function into two different files), * So this will return an array of locations. + * @param stagingMapPaths the list of `.map` file paths in the staging dir. Pass a pre-computed list + * (from {@link getStagingMapPaths}) when resolving many locations against the same staging dir to + * avoid re-walking the staging tree on every call. Falls back to walking the tree when omitted. */ public async getStagingLocations( sourceFilePath: string, @@ -105,7 +120,8 @@ export class LocationManager { sourceColumnIndex: number, sourceDirs: string[], stagingDir: string, - fileMappings: Array<{ src: string; dest: string }> + fileMappings: Array<{ src: string; dest: string }>, + stagingMapPaths?: string[] ): Promise<{ type: 'fileMap' | 'sourceDirs' | 'sourceMap'; locations: SourceLocation[] }> { sourceFilePath = s`${sourceFilePath}`; @@ -114,10 +130,7 @@ export class LocationManager { //look through the sourcemaps in the staging folder for any instances of this source location let locations = await this.sourceMapManager.getGeneratedLocations( - glob.sync('**/*.map', { - cwd: stagingDir, - absolute: true - }), + stagingMapPaths ?? this.getStagingMapPaths(stagingDir), { filePath: sourceFilePath, lineNumber: sourceLineNumber,