Skip to content

Commit

Permalink
perf(ngcc): link segment markers for faster traversal (#36027)
Browse files Browse the repository at this point in the history 
The merging algorithm needs to find, for a given segment, what the next
segment in the source file is. This change modifies the `generatedSegment`
properties in the mappings so that they have a link directly to the following
segment.

PR Close #36027
  • Loading branch information
@petebacondarwin @AndrewKushnir
petebacondarwin authored and AndrewKushnir committed Mar 13, 2020
1 parent daa2a08 commit a0fa63b
Show file tree
Hide file tree
Showing 4 changed files with 231 additions and 212 deletions.
3 changes: 2 additions & 1 deletion packages/compiler-cli/ngcc/src/sourcemaps/segment_marker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
export interface SegmentMarker {
readonly line: number;
readonly column: number;
next: SegmentMarker|undefined;
}

/**
Expand Down Expand Up @@ -64,5 +65,5 @@ export function offsetSegment(
line--;
}
const column = newPos - startOfLinePositions[line];
return {line, column};
return {line, column, next: undefined};
}
41 changes: 29 additions & 12 deletions packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export class SourceFile {
*/
private flattenMappings(): Mapping[] {
const mappings = parseMappings(this.rawMap, this.sources);
const originalSegmentsBySource = extractOriginalSegments(mappings);
ensureOriginalSegmentLinks(mappings);
const flattenedMappings: Mapping[] = [];
for (let mappingIndex = 0; mappingIndex < mappings.length; mappingIndex++) {
const aToBmapping = mappings[mappingIndex];
Expand Down Expand Up @@ -120,13 +120,8 @@ export class SourceFile {
// For mapping [0,0] the incoming start and end are 0 and 2 (i.e. the range a, b, c)
// For mapping [4,2] the incoming start and end are 2 and 5 (i.e. the range c, d, e, f)
//

const originalSegments = originalSegmentsBySource.get(bSource) !;
const incomingStart = aToBmapping.originalSegment;
const incomingEndIndex = originalSegments.indexOf(incomingStart) + 1;
const incomingEnd = incomingEndIndex < originalSegments.length ?
originalSegments[incomingEndIndex] :
undefined;
const incomingEnd = incomingStart.next;

// The `outgoingStartIndex` and `outgoingEndIndex` are the indices of the range of mappings
// that leave `b` that we are interested in merging with the aToBmapping.
Expand Down Expand Up @@ -330,12 +325,19 @@ export function parseMappings(
}
const generatedColumn = rawMapping[0];
const name = rawMapping.length === 5 ? rawMap.names[rawMapping[4]] : undefined;
const mapping: Mapping = {
generatedSegment: {line: generatedLine, column: generatedColumn},
originalSource,
originalSegment: {line: rawMapping[2] !, column: rawMapping[3] !}, name
const line = rawMapping[2] !;
const column = rawMapping[3] !;
const generatedSegment: SegmentMarker = {
line: generatedLine,
column: generatedColumn,
next: undefined,
};
const originalSegment: SegmentMarker = {
line,
column,
next: undefined,
};
mappings.push(mapping);
mappings.push({name, generatedSegment, originalSegment, originalSource});
}
}
}
Expand Down Expand Up @@ -364,6 +366,21 @@ export function extractOriginalSegments(mappings: Mapping[]): Map<SourceFile, Se
return originalSegments;
}

/**
* Update the original segments of each of the given `mappings` to include a link to the next
* segment in the source file.
*
* @param mappings the mappings whose segments should be updated
*/
export function ensureOriginalSegmentLinks(mappings: Mapping[]): void {
const segmentsBySource = extractOriginalSegments(mappings);
segmentsBySource.forEach(markers => {
for (let i = 0; i < markers.length - 1; i++) {
markers[i].next = markers[i + 1];
}
});
}

export function computeStartOfLinePositions(str: string) {
// The `1` is to indicate a newline character between the lines.
// Note that in the actual contents there could be more than one character that indicates a
Expand Down
160 changes: 63 additions & 97 deletions packages/compiler-cli/ngcc/test/sourcemaps/segment_marker_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,122 +11,88 @@ import {computeStartOfLinePositions} from '../../src/sourcemaps/source_file';
describe('SegmentMarker utils', () => {
describe('compareSegments()', () => {
it('should return 0 if the segments are the same', () => {
expect(compareSegments({line: 0, column: 0}, {line: 0, column: 0})).toEqual(0);
expect(compareSegments({line: 123, column: 0}, {line: 123, column: 0})).toEqual(0);
expect(compareSegments({line: 0, column: 45}, {line: 0, column: 45})).toEqual(0);
expect(compareSegments({line: 123, column: 45}, {line: 123, column: 45})).toEqual(0);
});

it('should return a negative number if the first segment is before the second segment', () => {
expect(compareSegments({line: 0, column: 0}, {line: 0, column: 45})).toBeLessThan(0);
expect(compareSegments({line: 123, column: 0}, {line: 123, column: 45})).toBeLessThan(0);
expect(compareSegments({line: 13, column: 45}, {line: 123, column: 45})).toBeLessThan(0);
expect(compareSegments({line: 13, column: 45}, {line: 123, column: 9})).toBeLessThan(0);
});

it('should return a positive number if the first segment is after the second segment', () => {
expect(compareSegments({line: 0, column: 45}, {line: 0, column: 0})).toBeGreaterThan(0);
expect(compareSegments({line: 123, column: 45}, {line: 123, column: 0})).toBeGreaterThan(0);
expect(compareSegments({line: 123, column: 45}, {line: 13, column: 45})).toBeGreaterThan(0);
expect(compareSegments({line: 123, column: 9}, {line: 13, column: 45})).toBeGreaterThan(0);
});
});

describe('segmentDiff()', () => {
it('should return 0 if the segments are the same', () => {
const startOfLinePositions =
computeStartOfLinePositions('abcdef\nabcdefghj\nabcdefghijklm\nabcdef');
expect(segmentDiff(startOfLinePositions, {line: 0, column: 0}, {line: 0, column: 0}))
expect(compareSegments(
{line: 0, column: 0, next: undefined}, {line: 0, column: 0, next: undefined}))
.toEqual(0);
expect(segmentDiff(startOfLinePositions, {line: 3, column: 0}, {line: 3, column: 0}))
expect(compareSegments(
{line: 123, column: 0, next: undefined}, {line: 123, column: 0, next: undefined}))
.toEqual(0);
expect(segmentDiff(startOfLinePositions, {line: 0, column: 5}, {line: 0, column: 5}))
expect(compareSegments(
{line: 0, column: 45, next: undefined}, {line: 0, column: 45, next: undefined}))
.toEqual(0);
expect(segmentDiff(startOfLinePositions, {line: 3, column: 5}, {line: 3, column: 5}))
expect(
compareSegments(
{line: 123, column: 45, next: undefined}, {line: 123, column: 45, next: undefined}))
.toEqual(0);
});

it('should return the column difference if the markers are on the same line', () => {
const startOfLinePositions =
computeStartOfLinePositions('abcdef\nabcdefghj\nabcdefghijklm\nabcdef');
expect(segmentDiff(startOfLinePositions, {line: 0, column: 0}, {line: 0, column: 3}))
.toEqual(3);
expect(segmentDiff(startOfLinePositions, {line: 1, column: 1}, {line: 1, column: 5}))
.toEqual(4);
expect(segmentDiff(startOfLinePositions, {line: 2, column: 5}, {line: 2, column: 1}))
.toEqual(-4);
expect(segmentDiff(startOfLinePositions, {line: 3, column: 3}, {line: 3, column: 0}))
.toEqual(-3);
it('should return a negative number if the first segment is before the second segment', () => {
expect(compareSegments(
{line: 0, column: 0, next: undefined}, {line: 0, column: 45, next: undefined}))
.toBeLessThan(0);
expect(compareSegments(
{line: 123, column: 0, next: undefined}, {line: 123, column: 45, next: undefined}))
.toBeLessThan(0);
expect(compareSegments(
{line: 13, column: 45, next: undefined}, {line: 123, column: 45, next: undefined}))
.toBeLessThan(0);
expect(compareSegments(
{line: 13, column: 45, next: undefined}, {line: 123, column: 9, next: undefined}))
.toBeLessThan(0);
});

it('should return the number of actual characters difference (including newline markers) if not on the same line',
() => {
let startOfLinePositions: number[];

startOfLinePositions = computeStartOfLinePositions('A12345\nB123456789');
expect(segmentDiff(startOfLinePositions, {line: 0, column: 0}, {line: 1, column: 0}))
.toEqual(6 + 1);

startOfLinePositions = computeStartOfLinePositions('012A45\n01234B6789');
expect(segmentDiff(startOfLinePositions, {line: 0, column: 3}, {line: 1, column: 5}))
.toEqual(3 + 1 + 5);

startOfLinePositions =
computeStartOfLinePositions('012345\n012345A789\n01234567\nB123456');
expect(segmentDiff(startOfLinePositions, {line: 1, column: 6}, {line: 3, column: 0}))
.toEqual(4 + 1 + 8 + 1 + 0);

startOfLinePositions =
computeStartOfLinePositions('012345\nA123456789\n01234567\n012B456');
expect(segmentDiff(startOfLinePositions, {line: 1, column: 0}, {line: 3, column: 3}))
.toEqual(10 + 1 + 8 + 1 + 3);

startOfLinePositions =
computeStartOfLinePositions('012345\nB123456789\nA1234567\n0123456');
expect(segmentDiff(startOfLinePositions, {line: 2, column: 0}, {line: 1, column: 0}))
.toEqual(0 - 1 - 10 + 0);

startOfLinePositions =
computeStartOfLinePositions('012345\n0123B56789\n01234567\n012A456');
expect(segmentDiff(startOfLinePositions, {line: 3, column: 3}, {line: 1, column: 4}))
.toEqual(-3 - 1 - 8 - 1 - 10 + 4);

startOfLinePositions =
computeStartOfLinePositions('B12345\n0123456789\n0123A567\n0123456');
expect(segmentDiff(startOfLinePositions, {line: 2, column: 4}, {line: 0, column: 0}))
.toEqual(-4 - 1 - 10 - 1 - 6 + 0);

startOfLinePositions =
computeStartOfLinePositions('0123B5\n0123456789\nA1234567\n0123456');
expect(segmentDiff(startOfLinePositions, {line: 2, column: 0}, {line: 0, column: 4}))
.toEqual(0 - 1 - 10 - 1 - 6 + 4);
});
it('should return a positive number if the first segment is after the second segment', () => {
expect(compareSegments(
{line: 0, column: 45, next: undefined}, {line: 0, column: 0, next: undefined}))
.toBeGreaterThan(0);
expect(compareSegments(
{line: 123, column: 45, next: undefined}, {line: 123, column: 0, next: undefined}))
.toBeGreaterThan(0);
expect(compareSegments(
{line: 123, column: 45, next: undefined}, {line: 13, column: 45, next: undefined}))
.toBeGreaterThan(0);
expect(compareSegments(
{line: 123, column: 9, next: undefined}, {line: 13, column: 45, next: undefined}))
.toBeGreaterThan(0);
});
});

describe('offsetSegment()', () => {
it('should return an identical marker if offset is 0', () => {
const startOfLinePositions =
computeStartOfLinePositions('012345\n0123456789\r\n01234567\n0123456');
const marker = {line: 2, column: 3};
computeStartOfLinePositions('012345\n0123456789\r\n012*4567\n0123456');
const marker = {line: 2, column: 3, next: undefined};
expect(offsetSegment(startOfLinePositions, marker, 0)).toBe(marker);
});

it('should return a new marker offset by the given chars', () => {
const startOfLinePositions =
computeStartOfLinePositions('012345\n0123456789\r\n012*4567\n0123456');
const marker = {line: 2, column: 3};
expect(offsetSegment(startOfLinePositions, marker, 1)).toEqual({line: 2, column: 4});
expect(offsetSegment(startOfLinePositions, marker, 2)).toEqual({line: 2, column: 5});
expect(offsetSegment(startOfLinePositions, marker, 4)).toEqual({line: 2, column: 7});
expect(offsetSegment(startOfLinePositions, marker, 6)).toEqual({line: 3, column: 0});
expect(offsetSegment(startOfLinePositions, marker, 8)).toEqual({line: 3, column: 2});
expect(offsetSegment(startOfLinePositions, marker, 20)).toEqual({line: 3, column: 14});
expect(offsetSegment(startOfLinePositions, marker, -1)).toEqual({line: 2, column: 2});
expect(offsetSegment(startOfLinePositions, marker, -2)).toEqual({line: 2, column: 1});
expect(offsetSegment(startOfLinePositions, marker, -3)).toEqual({line: 2, column: 0});
expect(offsetSegment(startOfLinePositions, marker, -4)).toEqual({line: 1, column: 10});
expect(offsetSegment(startOfLinePositions, marker, -6)).toEqual({line: 1, column: 8});
expect(offsetSegment(startOfLinePositions, marker, -16)).toEqual({line: 0, column: 5});
const marker = {line: 2, column: 3, next: undefined};
expect(offsetSegment(startOfLinePositions, marker, 1))
.toEqual({line: 2, column: 4, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, 2))
.toEqual({line: 2, column: 5, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, 4))
.toEqual({line: 2, column: 7, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, 6))
.toEqual({line: 3, column: 0, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, 8))
.toEqual({line: 3, column: 2, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, 20))
.toEqual({line: 3, column: 14, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -1))
.toEqual({line: 2, column: 2, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -2))
.toEqual({line: 2, column: 1, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -3))
.toEqual({line: 2, column: 0, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -4))
.toEqual({line: 1, column: 10, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -6))
.toEqual({line: 1, column: 8, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -16))
.toEqual({line: 0, column: 5, next: undefined});
});
});
});
Loading

0 comments on commit a0fa63b

Please sign in to comment.