From e8900824dd586c6bc1fda93f87d4f3bfcc38caeb Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 11 Mar 2020 12:17:36 +0000 Subject: [PATCH] perf(ngcc): use line start positions for computing offsets in source-map flattening (#36027) By computing and caching the start of each line, rather than the length of each line, we can save a lot of duplicated computation in the `segmentDiff()` and `offsetSegment()` functions. PR Close #36027 --- .../ngcc/src/sourcemaps/segment_marker.ts | 48 +++------ .../ngcc/src/sourcemaps/source_file.ts | 30 ++++-- .../test/sourcemaps/segment_marker_spec.ts | 102 +++++++++++------- .../ngcc/test/sourcemaps/source_file_spec.ts | 24 ++--- 4 files changed, 112 insertions(+), 92 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/sourcemaps/segment_marker.ts b/packages/compiler-cli/ngcc/src/sourcemaps/segment_marker.ts index 1a64748792..44126dfc4f 100644 --- a/packages/compiler-cli/ngcc/src/sourcemaps/segment_marker.ts +++ b/packages/compiler-cli/ngcc/src/sourcemaps/segment_marker.ts @@ -28,61 +28,41 @@ export function compareSegments(a: SegmentMarker, b: SegmentMarker): number { return a.line === b.line ? a.column - b.column : a.line - b.line; } -// 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 newline -// - e.g. \r\n - but that is not important here since segment-markers are in line/column pairs and -// so differences in length due to extra `\r` characters do not affect the algorithms. -const NEWLINE_MARKER_OFFSET = 1; - /** * Compute the difference between two segment markers in a source file. * - * @param lineLengths the lengths of each line of content of the source file where we are computing - * the difference + * @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(lineLengths: number[], a: SegmentMarker, b: SegmentMarker) { - let diff = b.column - a.column; - - // Deal with `a` being before `b` - for (let lineIndex = a.line; lineIndex < b.line; lineIndex++) { - diff += lineLengths[lineIndex] + NEWLINE_MARKER_OFFSET; - } - - // Deal with `a` being after `b` - for (let lineIndex = a.line - 1; lineIndex >= b.line; lineIndex--) { - // The `+ 1` is the newline character between the lines - diff -= lineLengths[lineIndex] + NEWLINE_MARKER_OFFSET; - } - return diff; +export function segmentDiff(startOfLinePositions: number[], a: SegmentMarker, b: SegmentMarker) { + return startOfLinePositions[b.line] - startOfLinePositions[a.line] + b.column - a.column; } /** * Return a new segment-marker that is offset by the given number of characters. * - * @param lineLengths The length of each line in the source file whose segment-marker we are - * offsetting. - * @param marker The segment to offset. - * @param offset The number of character to offset by. + * @param startOfLinePositions the position of the start of each line of content of the source file + * whose segment-marker we are offsetting. + * @param marker the segment to offset. + * @param offset the number of character to offset by. */ -export function offsetSegment(lineLengths: number[], marker: SegmentMarker, offset: number) { +export function offsetSegment( + startOfLinePositions: number[], marker: SegmentMarker, offset: number) { if (offset === 0) { return marker; } let line = marker.line; - let column = marker.column + offset; - - while (line < lineLengths.length - 1 && column > lineLengths[line]) { - column -= lineLengths[line] + NEWLINE_MARKER_OFFSET; + const newPos = startOfLinePositions[line] + marker.column + offset; + while (line < startOfLinePositions.length - 1 && startOfLinePositions[line + 1] <= newPos) { line++; } - while (line > 0 && column < 0) { + while (line > 0 && startOfLinePositions[line] > newPos) { line--; - column += lineLengths[line] + NEWLINE_MARKER_OFFSET; } - + const column = newPos - startOfLinePositions[line]; return {line, column}; } diff --git a/packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts b/packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts index 9c1faf5bd0..136531231e 100644 --- a/packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts +++ b/packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts @@ -24,7 +24,7 @@ export class SourceFile { * pure original source files). */ readonly flattenedMappings: Mapping[]; - readonly lineLengths: number[]; + readonly startOfLinePositions: number[]; constructor( /** The path to this source file. */ @@ -38,7 +38,7 @@ export class SourceFile { /** Any source files referenced by the raw source map associated with this source file. */ readonly sources: (SourceFile|null)[]) { this.contents = removeSourceMapComments(contents); - this.lineLengths = computeLineLengths(this.contents); + this.startOfLinePositions = computeStartOfLinePositions(this.contents); this.flattenedMappings = this.flattenMappings(); } @@ -282,11 +282,13 @@ 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.lineLengths, ab.originalSegment, bc.generatedSegment); + const diff = + segmentDiff(ab.originalSource.startOfLinePositions, ab.originalSegment, bc.generatedSegment); if (diff > 0) { return { name, - generatedSegment: offsetSegment(generatedSource.lineLengths, ab.generatedSegment, diff), + generatedSegment: + offsetSegment(generatedSource.startOfLinePositions, ab.generatedSegment, diff), originalSource: bc.originalSource, originalSegment: bc.originalSegment, }; @@ -295,7 +297,8 @@ export function mergeMappings(generatedSource: SourceFile, ab: Mapping, bc: Mapp name, generatedSegment: ab.generatedSegment, originalSource: bc.originalSource, - originalSegment: offsetSegment(bc.originalSource.lineLengths, bc.originalSegment, -diff), + originalSegment: + offsetSegment(bc.originalSource.startOfLinePositions, bc.originalSegment, -diff), }; } } @@ -361,6 +364,21 @@ export function extractOriginalSegments(mappings: Mapping[]): Map s.length); } diff --git a/packages/compiler-cli/ngcc/test/sourcemaps/segment_marker_spec.ts b/packages/compiler-cli/ngcc/test/sourcemaps/segment_marker_spec.ts index 9dc9b5e49c..a2ffc57e0d 100644 --- a/packages/compiler-cli/ngcc/test/sourcemaps/segment_marker_spec.ts +++ b/packages/compiler-cli/ngcc/test/sourcemaps/segment_marker_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import {compareSegments, offsetSegment, segmentDiff} from '../../src/sourcemaps/segment_marker'; -import {computeLineLengths} from '../../src/sourcemaps/source_file'; +import {computeStartOfLinePositions} from '../../src/sourcemaps/source_file'; describe('SegmentMarker utils', () => { describe('compareSegments()', () => { @@ -34,77 +34,99 @@ describe('SegmentMarker utils', () => { describe('segmentDiff()', () => { it('should return 0 if the segments are the same', () => { - const lineLengths = computeLineLengths('abcdef\nabcdefghj\nabcdefghijklm\nabcdef'); - expect(segmentDiff(lineLengths, {line: 0, column: 0}, {line: 0, column: 0})).toEqual(0); - expect(segmentDiff(lineLengths, {line: 3, column: 0}, {line: 3, column: 0})).toEqual(0); - expect(segmentDiff(lineLengths, {line: 0, column: 5}, {line: 0, column: 5})).toEqual(0); - expect(segmentDiff(lineLengths, {line: 3, column: 5}, {line: 3, column: 5})).toEqual(0); + const startOfLinePositions = + computeStartOfLinePositions('abcdef\nabcdefghj\nabcdefghijklm\nabcdef'); + expect(segmentDiff(startOfLinePositions, {line: 0, column: 0}, {line: 0, column: 0})) + .toEqual(0); + expect(segmentDiff(startOfLinePositions, {line: 3, column: 0}, {line: 3, column: 0})) + .toEqual(0); + expect(segmentDiff(startOfLinePositions, {line: 0, column: 5}, {line: 0, column: 5})) + .toEqual(0); + expect(segmentDiff(startOfLinePositions, {line: 3, column: 5}, {line: 3, column: 5})) + .toEqual(0); }); it('should return the column difference if the markers are on the same line', () => { - const lineLengths = computeLineLengths('abcdef\nabcdefghj\nabcdefghijklm\nabcdef'); - expect(segmentDiff(lineLengths, {line: 0, column: 0}, {line: 0, column: 3})).toEqual(3); - expect(segmentDiff(lineLengths, {line: 1, column: 1}, {line: 1, column: 5})).toEqual(4); - expect(segmentDiff(lineLengths, {line: 2, column: 5}, {line: 2, column: 1})).toEqual(-4); - expect(segmentDiff(lineLengths, {line: 3, column: 3}, {line: 3, column: 0})).toEqual(-3); + 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 the number of actual characters difference (including newlineLengths) if not on the same line', + it('should return the number of actual characters difference (including newline markers) if not on the same line', () => { - let lineLengths: number[]; + let startOfLinePositions: number[]; - lineLengths = computeLineLengths('A12345\nB123456789'); - expect(segmentDiff(lineLengths, {line: 0, column: 0}, {line: 1, column: 0})) + startOfLinePositions = computeStartOfLinePositions('A12345\nB123456789'); + expect(segmentDiff(startOfLinePositions, {line: 0, column: 0}, {line: 1, column: 0})) .toEqual(6 + 1); - lineLengths = computeLineLengths('012A45\n01234B6789'); - expect(segmentDiff(lineLengths, {line: 0, column: 3}, {line: 1, column: 5})) + startOfLinePositions = computeStartOfLinePositions('012A45\n01234B6789'); + expect(segmentDiff(startOfLinePositions, {line: 0, column: 3}, {line: 1, column: 5})) .toEqual(3 + 1 + 5); - lineLengths = computeLineLengths('012345\n012345A789\n01234567\nB123456'); - expect(segmentDiff(lineLengths, {line: 1, column: 6}, {line: 3, column: 0})) + startOfLinePositions = + computeStartOfLinePositions('012345\n012345A789\n01234567\nB123456'); + expect(segmentDiff(startOfLinePositions, {line: 1, column: 6}, {line: 3, column: 0})) .toEqual(4 + 1 + 8 + 1 + 0); - lineLengths = computeLineLengths('012345\nA123456789\n01234567\n012B456'); - expect(segmentDiff(lineLengths, {line: 1, column: 0}, {line: 3, column: 3})) + startOfLinePositions = + computeStartOfLinePositions('012345\nA123456789\n01234567\n012B456'); + expect(segmentDiff(startOfLinePositions, {line: 1, column: 0}, {line: 3, column: 3})) .toEqual(10 + 1 + 8 + 1 + 3); - lineLengths = computeLineLengths('012345\nB123456789\nA1234567\n0123456'); - expect(segmentDiff(lineLengths, {line: 2, column: 0}, {line: 1, column: 0})) + startOfLinePositions = + computeStartOfLinePositions('012345\nB123456789\nA1234567\n0123456'); + expect(segmentDiff(startOfLinePositions, {line: 2, column: 0}, {line: 1, column: 0})) .toEqual(0 - 1 - 10 + 0); - lineLengths = computeLineLengths('012345\n0123B56789\n01234567\n012A456'); - expect(segmentDiff(lineLengths, {line: 3, column: 3}, {line: 1, column: 4})) + startOfLinePositions = + computeStartOfLinePositions('012345\n0123B56789\n01234567\n012A456'); + expect(segmentDiff(startOfLinePositions, {line: 3, column: 3}, {line: 1, column: 4})) .toEqual(-3 - 1 - 8 - 1 - 10 + 4); - lineLengths = computeLineLengths('B12345\n0123456789\n0123A567\n0123456'); - expect(segmentDiff(lineLengths, {line: 2, column: 4}, {line: 0, column: 0})) + startOfLinePositions = + computeStartOfLinePositions('B12345\n0123456789\n0123A567\n0123456'); + expect(segmentDiff(startOfLinePositions, {line: 2, column: 4}, {line: 0, column: 0})) .toEqual(-4 - 1 - 10 - 1 - 6 + 0); - lineLengths = computeLineLengths('0123B5\n0123456789\nA1234567\n0123456'); - expect(segmentDiff(lineLengths, {line: 2, column: 0}, {line: 0, column: 4})) + startOfLinePositions = + computeStartOfLinePositions('0123B5\n0123456789\nA1234567\n0123456'); + expect(segmentDiff(startOfLinePositions, {line: 2, column: 0}, {line: 0, column: 4})) .toEqual(0 - 1 - 10 - 1 - 6 + 4); }); }); describe('offsetSegment()', () => { it('should return an identical marker if offset is 0', () => { - const lineLengths = computeLineLengths('012345\n0123456789\n01234567\n0123456'); + const startOfLinePositions = + computeStartOfLinePositions('012345\n0123456789\r\n01234567\n0123456'); const marker = {line: 2, column: 3}; - expect(offsetSegment(lineLengths, marker, 0)).toBe(marker); + expect(offsetSegment(startOfLinePositions, marker, 0)).toBe(marker); }); it('should return a new marker offset by the given chars', () => { - const lineLengths = computeLineLengths('012345\n0123456789\n012*4567\n0123456'); + const startOfLinePositions = + computeStartOfLinePositions('012345\n0123456789\r\n012*4567\n0123456'); const marker = {line: 2, column: 3}; - expect(offsetSegment(lineLengths, marker, 1)).toEqual({line: 2, column: 4}); - expect(offsetSegment(lineLengths, marker, 2)).toEqual({line: 2, column: 5}); - expect(offsetSegment(lineLengths, marker, 4)).toEqual({line: 2, column: 7}); - expect(offsetSegment(lineLengths, marker, 8)).toEqual({line: 3, column: 2}); - expect(offsetSegment(lineLengths, marker, -1)).toEqual({line: 2, column: 2}); - expect(offsetSegment(lineLengths, marker, -2)).toEqual({line: 2, column: 1}); - expect(offsetSegment(lineLengths, marker, -4)).toEqual({line: 1, column: 10}); - expect(offsetSegment(lineLengths, marker, -6)).toEqual({line: 1, column: 8}); + 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}); }); }); }); \ No newline at end of file diff --git a/packages/compiler-cli/ngcc/test/sourcemaps/source_file_spec.ts b/packages/compiler-cli/ngcc/test/sourcemaps/source_file_spec.ts index fe8b35cd47..18acb3f30b 100644 --- a/packages/compiler-cli/ngcc/test/sourcemaps/source_file_spec.ts +++ b/packages/compiler-cli/ngcc/test/sourcemaps/source_file_spec.ts @@ -11,7 +11,7 @@ import {absoluteFrom} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {RawSourceMap} from '../../src/sourcemaps/raw_source_map'; import {SegmentMarker} from '../../src/sourcemaps/segment_marker'; -import {Mapping, SourceFile, computeLineLengths, extractOriginalSegments, findLastMappingIndexBefore, parseMappings} from '../../src/sourcemaps/source_file'; +import {Mapping, SourceFile, computeStartOfLinePositions, extractOriginalSegments, findLastMappingIndexBefore, parseMappings} from '../../src/sourcemaps/source_file'; runInEachFileSystem(() => { describe('SourceFile and utilities', () => { @@ -482,17 +482,17 @@ runInEachFileSystem(() => { }); }); - describe('computeLineLengths()', () => { - it('should compute the length of each line in the given string', () => { - expect(computeLineLengths('')).toEqual([0]); - expect(computeLineLengths('abc')).toEqual([3]); - expect(computeLineLengths('\n')).toEqual([0, 0]); - expect(computeLineLengths('\n\n')).toEqual([0, 0, 0]); - expect(computeLineLengths('abc\n')).toEqual([3, 0]); - expect(computeLineLengths('\nabc')).toEqual([0, 3]); - expect(computeLineLengths('abc\ndefg')).toEqual([3, 4]); - expect(computeLineLengths('abc\r\n')).toEqual([3, 0]); - expect(computeLineLengths('abc\r\ndefg')).toEqual([3, 4]); + describe('computeStartOfLinePositions()', () => { + it('should compute the cumulative length of each line in the given string', () => { + expect(computeStartOfLinePositions('')).toEqual([0]); + expect(computeStartOfLinePositions('abc')).toEqual([0]); + expect(computeStartOfLinePositions('\n')).toEqual([0, 1]); + expect(computeStartOfLinePositions('\n\n')).toEqual([0, 1, 2]); + expect(computeStartOfLinePositions('abc\n')).toEqual([0, 4]); + expect(computeStartOfLinePositions('\nabc')).toEqual([0, 1]); + expect(computeStartOfLinePositions('abc\ndefg')).toEqual([0, 4]); + expect(computeStartOfLinePositions('abc\r\n')).toEqual([0, 4]); + expect(computeStartOfLinePositions('abc\r\ndefg')).toEqual([0, 4]); }); }); });