From 348ff0c8eafef51eeef6684038f4699d9ff0d6aa Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Mon, 9 Mar 2020 17:03:36 +0000 Subject: [PATCH] perf(ngcc): use binary search when flattening mappings (#36027) The `@angular/core` package has a large number of source files and mappings which exposed performance issues in the new source-map flattening algorithm. This change uses a binary search (rather than linear) when finding matching mappings to merge. Initial measurements indicate that this reduces processing time for `@angular/core` by about 50%. PR Close #36027 --- .../ngcc/src/sourcemaps/source_file.ts | 45 +++- .../ngcc/test/sourcemaps/source_file_spec.ts | 206 +++++++++++++++++- 2 files changed, 236 insertions(+), 15 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts b/packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts index e18d94ff80..dcc3a6418f 100644 --- a/packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts +++ b/packages/compiler-cli/ngcc/src/sourcemaps/source_file.ts @@ -147,16 +147,14 @@ export class SourceFile { // The range with `incomingStart` at 2 and `incomingEnd` at 5 has outgoing start mapping of // [1,0] and outgoing end mapping of [4, 6], which also includes [4, 3]. // - let outgoingStartIndex = findLastIndex( - bSource.flattenedMappings, - mapping => compareSegments(mapping.generatedSegment, incomingStart) <= 0); + let outgoingStartIndex = + findLastMappingIndexBefore(bSource.flattenedMappings, incomingStart, false, 0); if (outgoingStartIndex < 0) { outgoingStartIndex = 0; } const outgoingEndIndex = incomingEnd !== undefined ? - findLastIndex( - bSource.flattenedMappings, - mapping => compareSegments(mapping.generatedSegment, incomingEnd) < 0) : + findLastMappingIndexBefore( + bSource.flattenedMappings, incomingEnd, true, outgoingStartIndex) : bSource.flattenedMappings.length - 1; for (let bToCmappingIndex = outgoingStartIndex; bToCmappingIndex <= outgoingEndIndex; @@ -169,13 +167,38 @@ export class SourceFile { } } -function findLastIndex(items: T[], predicate: (item: T) => boolean): number { - for (let index = items.length - 1; index >= 0; index--) { - if (predicate(items[index])) { - return index; +/** + * + * @param mappings The collection of mappings whose segment-markers we are searching. + * @param marker The segment-marker to match against those of the given `mappings`. + * @param exclusive If exclusive then we must find a mapping with a segment-marker that is + * exclusively earlier than the given `marker`. + * If not exclusive then we can return the highest mappings with an equivalent segment-marker to the + * given `marker`. + * @param lowerIndex If provided, this is used as a hint that the marker we are searching for has an + * index that is no lower than this. + */ +export function findLastMappingIndexBefore( + mappings: Mapping[], marker: SegmentMarker, exclusive: boolean, lowerIndex: number): number { + let upperIndex = mappings.length - 1; + const test = exclusive ? -1 : 0; + + if (compareSegments(mappings[lowerIndex].generatedSegment, marker) > test) { + // Exit early since the marker is outside the allowed range of mappings. + return -1; + } + + let matchingIndex = -1; + while (lowerIndex <= upperIndex) { + const index = (upperIndex + lowerIndex) >> 1; + if (compareSegments(mappings[index].generatedSegment, marker) <= test) { + matchingIndex = index; + lowerIndex = index + 1; + } else { + upperIndex = index - 1; } } - return -1; + return matchingIndex; } /** 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 e1ef2bfb26..c7bc309345 100644 --- a/packages/compiler-cli/ngcc/test/sourcemaps/source_file_spec.ts +++ b/packages/compiler-cli/ngcc/test/sourcemaps/source_file_spec.ts @@ -10,15 +10,14 @@ import {encode} from 'sourcemap-codec'; import {absoluteFrom} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {RawSourceMap} from '../../src/sourcemaps/raw_source_map'; -import {SourceFile, computeLineLengths, extractOriginalSegments, parseMappings} from '../../src/sourcemaps/source_file'; +import {SegmentMarker} from '../../src/sourcemaps/segment_marker'; +import {Mapping, SourceFile, computeLineLengths, extractOriginalSegments, findLastMappingIndexBefore, parseMappings} from '../../src/sourcemaps/source_file'; runInEachFileSystem(() => { describe('SourceFile and utilities', () => { let _: typeof absoluteFrom; - beforeEach(() => { - _ = absoluteFrom; - }); + beforeEach(() => { _ = absoluteFrom; }); describe('parseMappings()', () => { it('should be an empty array for source files with no source map', () => { @@ -84,6 +83,205 @@ runInEachFileSystem(() => { }); }); + describe('findLastMappingIndexBefore', () => { + it('should find the highest mapping index that has a segment marker below the given one if there is not an exact match', + () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 40}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + + const marker: SegmentMarker = {line: 0, column: 35}; + const index = findLastMappingIndexBefore(mappings, marker, /* exclusive */ false, 0); + expect(index).toEqual(2); + }); + + it('should find the highest mapping index that has a segment marker (when there are duplicates) below the given one if there is not an exact match', + () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 30}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + + const marker: SegmentMarker = {line: 0, column: 35}; + const index = findLastMappingIndexBefore(mappings, marker, /* exclusive */ false, 0); + expect(index).toEqual(3); + }); + + it('should find the last mapping if the segment marker is higher than all of them', () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 40}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + + const marker: SegmentMarker = {line: 0, column: 60}; + + const index = findLastMappingIndexBefore(mappings, marker, /* exclusive */ false, 0); + expect(index).toEqual(4); + }); + + it('should return -1 if the segment marker is lower than all of them', () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 40}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + + const marker: SegmentMarker = {line: 0, column: 5}; + + const index = findLastMappingIndexBefore(mappings, marker, /* exclusive */ false, 0); + expect(index).toEqual(-1); + }); + + describe('[exact match inclusive]', () => { + it('should find the matching segment marker mapping index if there is only one of them', + () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 40}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + const index = findLastMappingIndexBefore(mappings, marker3, /* exclusive */ false, 0); + expect(index).toEqual(2); + }); + + it('should find the highest matching segment marker mapping index if there is more than one of them', + () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 30}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + const index = findLastMappingIndexBefore(mappings, marker3, /* exclusive */ false, 0); + expect(index).toEqual(3); + }); + }); + + describe('[exact match exclusive]', () => { + it('should find the preceding mapping index if there is a matching segment marker', () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 40}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + const index = findLastMappingIndexBefore(mappings, marker3, /* exclusive */ true, 0); + expect(index).toEqual(1); + }); + + it('should find the highest preceding mapping index if there is more than one matching segment marker', + () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 30}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + const index = findLastMappingIndexBefore(mappings, marker3, /* exclusive */ false, 0); + expect(index).toEqual(3); + }); + }); + + describe('[with lowerIndex hint', () => { + it('should find the highest mapping index above the lowerIndex hint that has a segment marker below the given one if there is not an exact match', + () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 40}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + + const marker: SegmentMarker = {line: 0, column: 35}; + const index = findLastMappingIndexBefore(mappings, marker, /* exclusive */ false, 1); + expect(index).toEqual(2); + }); + + it('should return the lowerIndex mapping index if there is a single exact match and we are not exclusive', + () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 40}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + + const marker: SegmentMarker = {line: 0, column: 30}; + const index = findLastMappingIndexBefore(mappings, marker, /* exclusive */ false, 2); + expect(index).toEqual(2); + }); + + it('should return the lowerIndex mapping index if there are multiple exact matches and we are not exclusive', + () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 30}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + + const marker: SegmentMarker = {line: 0, column: 30}; + const index = findLastMappingIndexBefore(mappings, marker, /* exclusive */ false, 3); + expect(index).toEqual(3); + }); + + it('should return -1 if the segment marker is lower than the lowerIndex hint', () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 40}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + + const marker: SegmentMarker = {line: 0, column: 25}; + + const index = findLastMappingIndexBefore(mappings, marker, /* exclusive */ false, 2); + expect(index).toEqual(-1); + }); + + it('should return -1 if the segment marker is equal to the lowerIndex hint and we are exclusive', + () => { + const marker5: SegmentMarker = {line: 0, column: 50}; + const marker4: SegmentMarker = {line: 0, column: 40}; + const marker3: SegmentMarker = {line: 0, column: 30}; + const marker2: SegmentMarker = {line: 0, column: 20}; + const marker1: SegmentMarker = {line: 0, column: 10}; + const mappings: Mapping[] = [marker1, marker2, marker3, marker4, marker5].map( + marker => ({ generatedSegment: marker } as Mapping)); + + const marker: SegmentMarker = {line: 0, column: 30}; + + const index = findLastMappingIndexBefore(mappings, marker, /* exclusive */ true, 2); + expect(index).toEqual(-1); + }); + }); + }); + describe('SourceFile', () => { describe('flattenedMappings', () => { it('should be an empty array for source files with no source map', () => {