Skip to content

Commit

Permalink
perf(ngcc): store the position of SegmentMarkers to avoid unnecessary…
Browse files Browse the repository at this point in the history 
… computation (#36027)

Previously, calculations related to the position of and difference between
SegmentMarkers required extensive computation based around the line,
line start positions and columns of each segment.

PR Close #36027
  • Loading branch information
@petebacondarwin @AndrewKushnir
petebacondarwin authored and AndrewKushnir committed Mar 13, 2020
1 parent a0fa63b commit 4ce9098
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 163 deletions.
28 changes: 8 additions & 20 deletions 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;
readonly position: number;
next: SegmentMarker|undefined;
}

Expand All @@ -26,20 +27,7 @@ export interface SegmentMarker {
* and zero if they are at the same position.
*/
export function compareSegments(a: SegmentMarker, b: SegmentMarker): number {
return a.line === b.line ? a.column - b.column : a.line - b.line;
}

/**
* Compute the difference between two segment markers in a source file.
*
* @param startOfLinePositions the position of the start of each line of content of the source file
* where we are computing the difference
* @param a the start marker
* @param b the end marker
* @returns the number of characters between the two segments `a` and `b`
*/
export function segmentDiff(startOfLinePositions: number[], a: SegmentMarker, b: SegmentMarker) {
return startOfLinePositions[b.line] - startOfLinePositions[a.line] + b.column - a.column;
return a.position - b.position;
}

/**
Expand All @@ -51,19 +39,19 @@ export function segmentDiff(startOfLinePositions: number[], a: SegmentMarker, b:
* @param offset the number of character to offset by.
*/
export function offsetSegment(
startOfLinePositions: number[], marker: SegmentMarker, offset: number) {
startOfLinePositions: number[], marker: SegmentMarker, offset: number): SegmentMarker {
if (offset === 0) {
return marker;
}

let line = marker.line;
const newPos = startOfLinePositions[line] + marker.column + offset;
while (line < startOfLinePositions.length - 1 && startOfLinePositions[line + 1] <= newPos) {
const position = marker.position + offset;
while (line < startOfLinePositions.length - 1 && startOfLinePositions[line + 1] <= position) {
line++;
}
while (line > 0 && startOfLinePositions[line] > newPos) {
while (line > 0 && startOfLinePositions[line] > position) {
line--;
}
const column = newPos - startOfLinePositions[line];
return {line, column, next: undefined};
const column = position - startOfLinePositions[line];
return {line, column, position, next: undefined};
}
12 changes: 7 additions & 5 deletions packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {removeComments, removeMapFileComments} from 'convert-source-map';
import {SourceMapMappings, SourceMapSegment, decode, encode} from 'sourcemap-codec';
import {AbsoluteFsPath, dirname, relative} from '../../../src/ngtsc/file_system';
import {RawSourceMap} from './raw_source_map';
import {SegmentMarker, compareSegments, offsetSegment, segmentDiff} from './segment_marker';
import {SegmentMarker, compareSegments, offsetSegment} from './segment_marker';

export function removeSourceMapComments(contents: string): string {
return removeMapFileComments(removeComments(contents)).replace(/\n\n$/, '\n');
Expand Down Expand Up @@ -89,7 +89,7 @@ export class SourceFile {
* source files with no transitive source maps.
*/
private flattenMappings(): Mapping[] {
const mappings = parseMappings(this.rawMap, this.sources);
const mappings = parseMappings(this.rawMap, this.sources, this.startOfLinePositions);
ensureOriginalSegmentLinks(mappings);
const flattenedMappings: Mapping[] = [];
for (let mappingIndex = 0; mappingIndex < mappings.length; mappingIndex++) {
Expand Down Expand Up @@ -277,8 +277,7 @@ export function mergeMappings(generatedSource: SourceFile, ab: Mapping, bc: Mapp
// segment-marker" of B->C (4*): `1 - 4 = -3`.
// Since it is negative we must increment the "generated segment-marker" with `3` to give [3,2].

const diff =
segmentDiff(ab.originalSource.startOfLinePositions, ab.originalSegment, bc.generatedSegment);
const diff = compareSegments(bc.generatedSegment, ab.originalSegment);
if (diff > 0) {
return {
name,
Expand All @@ -303,7 +302,8 @@ export function mergeMappings(generatedSource: SourceFile, ab: Mapping, bc: Mapp
* in the `sources` parameter.
*/
export function parseMappings(
rawMap: RawSourceMap | null, sources: (SourceFile | null)[]): Mapping[] {
rawMap: RawSourceMap | null, sources: (SourceFile | null)[],
generatedSourceStartOfLinePositions: number[]): Mapping[] {
if (rawMap === null) {
return [];
}
Expand All @@ -330,11 +330,13 @@ export function parseMappings(
const generatedSegment: SegmentMarker = {
line: generatedLine,
column: generatedColumn,
position: generatedSourceStartOfLinePositions[generatedLine] + generatedColumn,
next: undefined,
};
const originalSegment: SegmentMarker = {
line,
column,
position: originalSource.startOfLinePositions[line] + column,
next: undefined,
};
mappings.push({name, generatedSegment, originalSegment, originalSource});
Expand Down
69 changes: 40 additions & 29 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 @@ -5,54 +5,65 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {compareSegments, offsetSegment, segmentDiff} from '../../src/sourcemaps/segment_marker';
import {compareSegments, offsetSegment} from '../../src/sourcemaps/segment_marker';
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, next: undefined}, {line: 0, column: 0, next: undefined}))
{line: 0, column: 0, position: 0, next: undefined},
{line: 0, column: 0, position: 0, next: undefined}))
.toEqual(0);
expect(compareSegments(
{line: 123, column: 0, next: undefined}, {line: 123, column: 0, next: undefined}))
{line: 123, column: 0, position: 200, next: undefined},
{line: 123, column: 0, position: 200, next: undefined}))
.toEqual(0);
expect(compareSegments(
{line: 0, column: 45, next: undefined}, {line: 0, column: 45, next: undefined}))
{line: 0, column: 45, position: 45, next: undefined},
{line: 0, column: 45, position: 45, next: undefined}))
.toEqual(0);
expect(
compareSegments(
{line: 123, column: 45, next: undefined}, {line: 123, column: 45, next: undefined}))
expect(compareSegments(
{line: 123, column: 45, position: 245, next: undefined},
{line: 123, column: 45, position: 245, next: undefined}))
.toEqual(0);
});

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}))
{line: 0, column: 0, position: 0, next: undefined},
{line: 0, column: 45, position: 45, next: undefined}))
.toBeLessThan(0);
expect(compareSegments(
{line: 123, column: 0, next: undefined}, {line: 123, column: 45, next: undefined}))
{line: 123, column: 0, position: 200, next: undefined},
{line: 123, column: 45, position: 245, next: undefined}))
.toBeLessThan(0);
expect(compareSegments(
{line: 13, column: 45, next: undefined}, {line: 123, column: 45, next: undefined}))
{line: 13, column: 45, position: 75, next: undefined},
{line: 123, column: 45, position: 245, next: undefined}))
.toBeLessThan(0);
expect(compareSegments(
{line: 13, column: 45, next: undefined}, {line: 123, column: 9, next: undefined}))
{line: 13, column: 45, position: 75, next: undefined},
{line: 123, column: 9, position: 209, next: undefined}))
.toBeLessThan(0);
});

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}))
{line: 0, column: 45, position: 45, next: undefined},
{line: 0, column: 0, position: 0, next: undefined}))
.toBeGreaterThan(0);
expect(compareSegments(
{line: 123, column: 45, next: undefined}, {line: 123, column: 0, next: undefined}))
{line: 123, column: 45, position: 245, next: undefined},
{line: 123, column: 0, position: 200, next: undefined}))
.toBeGreaterThan(0);
expect(compareSegments(
{line: 123, column: 45, next: undefined}, {line: 13, column: 45, next: undefined}))
{line: 123, column: 45, position: 245, next: undefined},
{line: 13, column: 45, position: 75, next: undefined}))
.toBeGreaterThan(0);
expect(compareSegments(
{line: 123, column: 9, next: undefined}, {line: 13, column: 45, next: undefined}))
{line: 123, column: 9, position: 209, next: undefined},
{line: 13, column: 45, position: 75, next: undefined}))
.toBeGreaterThan(0);
});
});
Expand All @@ -61,38 +72,38 @@ describe('SegmentMarker utils', () => {
it('should return an identical marker if offset is 0', () => {
const startOfLinePositions =
computeStartOfLinePositions('012345\n0123456789\r\n012*4567\n0123456');
const marker = {line: 2, column: 3, next: undefined};
const marker = {line: 2, column: 3, position: 20, 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, next: undefined};
const marker = {line: 2, column: 3, position: 21, next: undefined};
expect(offsetSegment(startOfLinePositions, marker, 1))
.toEqual({line: 2, column: 4, next: undefined});
.toEqual({line: 2, column: 4, position: 22, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, 2))
.toEqual({line: 2, column: 5, next: undefined});
.toEqual({line: 2, column: 5, position: 23, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, 4))
.toEqual({line: 2, column: 7, next: undefined});
.toEqual({line: 2, column: 7, position: 25, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, 6))
.toEqual({line: 3, column: 0, next: undefined});
.toEqual({line: 3, column: 0, position: 27, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, 8))
.toEqual({line: 3, column: 2, next: undefined});
.toEqual({line: 3, column: 2, position: 29, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, 20))
.toEqual({line: 3, column: 14, next: undefined});
.toEqual({line: 3, column: 14, position: 41, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -1))
.toEqual({line: 2, column: 2, next: undefined});
.toEqual({line: 2, column: 2, position: 20, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -2))
.toEqual({line: 2, column: 1, next: undefined});
.toEqual({line: 2, column: 1, position: 19, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -3))
.toEqual({line: 2, column: 0, next: undefined});
.toEqual({line: 2, column: 0, position: 18, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -4))
.toEqual({line: 1, column: 10, next: undefined});
.toEqual({line: 1, column: 10, position: 17, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -6))
.toEqual({line: 1, column: 8, next: undefined});
.toEqual({line: 1, column: 8, position: 15, next: undefined});
expect(offsetSegment(startOfLinePositions, marker, -16))
.toEqual({line: 0, column: 5, next: undefined});
.toEqual({line: 0, column: 5, position: 5, next: undefined});
});
});
});
Loading

0 comments on commit 4ce9098

Please sign in to comment.