Skip to content
Open
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
22 changes: 22 additions & 0 deletions src/managers/BreakpointManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(<any>{
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`);
Expand Down
7 changes: 6 additions & 1 deletion src/managers/BreakpointManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,10 @@ export class BreakpointManager {
private async getBreakpointWork(project: Project, willInjectStop = false) {
let result = {} as Record<string, Array<BreakpointWorkItem>>;

//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) {
Expand All @@ -438,7 +442,8 @@ export class BreakpointManager {
project.rootDir
],
project.stagingDir,
project.fileMappings
project.fileMappings,
stagingMapPaths
);

for (let stagingLocation of stagingLocationsResult.locations) {
Expand Down
49 changes: 49 additions & 0 deletions src/managers/LocationManager.spec.ts
Original file line number Diff line number Diff line change
@@ -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`;
Expand All @@ -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 () => {
Expand Down
23 changes: 18 additions & 5 deletions src/managers/LocationManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,19 +93,35 @@ 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,
sourceLineNumber: number,
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}`;
Expand All @@ -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,
Expand Down