From c65ac7fbadb26d2be891e27d57fe9b5738c2b6b3 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sat, 6 Apr 2019 15:35:40 +0100 Subject: [PATCH] perf(ivy): ngcc - exit early if the targeted package has been compiled (#29740) Previously we always walked the whole folder tree looking for entry-points before we tested whether a target package had been processed already. This could take >10secs! This commit does a quick check of the target package before doing the full walk which brings down the execution time for ngcc in this case dramatically. ``` $ time ./node_modules/.bin/ivy-ngcc -t @angular/common/http/testing Compiling @angular/core : fesm2015 as esm2015 Compiling @angular/core : fesm5 as esm5 Compiling @angular/core : esm2015 as esm2015 Compiling @angular/core : esm5 as esm5 Compiling @angular/common/http : fesm2015 as esm2015 Compiling @angular/common/http : fesm5 as esm5 Compiling @angular/common/http : esm2015 as esm2015 Compiling @angular/common/http : esm5 as esm5 Compiling @angular/common/http/testing : fesm2015 as esm2015 Compiling @angular/common/http/testing : fesm5 as esm5 Compiling @angular/common/http/testing : esm2015 as esm2015 Compiling @angular/common/http/testing : esm5 as esm5 real 0m19.766s user 0m28.533s sys 0m2.262s ``` ``` $ time ./node_modules/.bin/ivy-ngcc -t @angular/common/http/testing The target entry-point has already been processed real 0m0.666s user 0m0.605s sys 0m0.113s ``` PR Close #29740 --- packages/compiler-cli/ngcc/src/main.ts | 33 ++++++++ .../ngcc/test/integration/ngcc_spec.ts | 80 ++++++++++++++++++- 2 files changed, 111 insertions(+), 2 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index 4f9825c858..35cec79ba7 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/packages/compiler-cli/ngcc/src/main.ts @@ -82,6 +82,14 @@ export function mainNgcc({basePath, targetEntryPointPath, const absoluteTargetEntryPointPath = targetEntryPointPath ? AbsoluteFsPath.from(resolve(basePath, targetEntryPointPath)) : undefined; + + if (absoluteTargetEntryPointPath && + hasProcessedTargetEntryPoint( + absoluteTargetEntryPointPath, propertiesToConsider, compileAllFormats)) { + logger.info('The target entry-point has already been processed'); + return; + } + const {entryPoints} = finder.findEntryPoints(AbsoluteFsPath.from(basePath), absoluteTargetEntryPointPath); @@ -167,3 +175,28 @@ export function mainNgcc({basePath, targetEntryPointPath, function getFileWriter(createNewEntryPointFormats: boolean): FileWriter { return createNewEntryPointFormats ? new NewEntryPointFileWriter() : new InPlaceFileWriter(); } + +function hasProcessedTargetEntryPoint( + targetPath: AbsoluteFsPath, propertiesToConsider: string[], compileAllFormats: boolean) { + const packageJsonPath = AbsoluteFsPath.from(resolve(targetPath, 'package.json')); + const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8')); + + for (const property of propertiesToConsider) { + if (packageJson[property]) { + // Here is a property that should be processed + if (hasBeenProcessed(packageJson, property as EntryPointJsonProperty)) { + if (!compileAllFormats) { + // It has been processed and we only need one, so we are done. + return true; + } + } else { + // It has not been processed but we need all of them, so we are done. + return false; + } + } + } + // Either all formats need to be compiled and there were none that were unprocessed, + // Or only the one matching format needs to be compiled but there was at least one matching + // property before the first processed format that was unprocessed. + return true; +} diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index 50838db295..6b72e3a47d 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -6,16 +6,20 @@ * found in the LICENSE file at https://angular.io/license */ -import {existsSync, readFileSync, readdirSync, statSync} from 'fs'; +import {AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/path'; +import {existsSync, readFileSync, readdirSync, statSync, writeFileSync} from 'fs'; import * as mockFs from 'mock-fs'; import {join} from 'path'; const Module = require('module'); import {getAngularPackagesFromRunfiles, resolveNpmTreeArtifact} from '../../../test/runfile_helpers'; import {mainNgcc} from '../../src/main'; -import {EntryPointPackageJson} from '../../src/packages/entry_point'; +import {markAsProcessed} from '../../src/packages/build_marker'; +import {EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTIES} from '../../src/packages/entry_point'; import {MockLogger} from '../helpers/mock_logger'; +const _ = AbsoluteFsPath.from; + describe('ngcc main()', () => { beforeEach(createMockFileSystem); afterEach(restoreRealFileSystem); @@ -86,6 +90,78 @@ describe('ngcc main()', () => { }); }); + describe('early skipping of target entry-point', () => { + describe('[compileAllFormats === true]', () => { + it('should skip all processing if all the properties are marked as processed', () => { + const logger = new MockLogger(); + markPropertiesAsProcessed('@angular/common/http/testing', SUPPORTED_FORMAT_PROPERTIES); + mainNgcc({ + basePath: '/node_modules', + targetEntryPointPath: '@angular/common/http/testing', logger, + }); + expect(logger.logs.info).toContain(['The target entry-point has already been processed']); + }); + + it('should process the target if any `propertyToConsider` is not marked as processed', () => { + const logger = new MockLogger(); + markPropertiesAsProcessed('@angular/common/http/testing', ['esm2015', 'fesm2015']); + mainNgcc({ + basePath: '/node_modules', + targetEntryPointPath: '@angular/common/http/testing', + propertiesToConsider: ['fesm2015', 'esm5', 'esm2015'], logger, + }); + expect(logger.logs.info).not.toContain([ + 'The target entry-point has already been processed' + ]); + }); + }); + + describe('[compileAllFormats === false]', () => { + it('should process the target if the first matching `propertyToConsider` is not marked as processed', + () => { + const logger = new MockLogger(); + markPropertiesAsProcessed('@angular/common/http/testing', ['esm2015']); + mainNgcc({ + basePath: '/node_modules', + targetEntryPointPath: '@angular/common/http/testing', + propertiesToConsider: ['esm5', 'esm2015'], + compileAllFormats: false, logger, + }); + + expect(logger.logs.info).not.toContain([ + 'The target entry-point has already been processed' + ]); + }); + + it('should skip all processing if the first matching `propertyToConsider` is marked as processed', + () => { + const logger = new MockLogger(); + markPropertiesAsProcessed('@angular/common/http/testing', ['esm2015']); + mainNgcc({ + basePath: '/node_modules', + targetEntryPointPath: '@angular/common/http/testing', + // Simulate a property that does not exist on the package.json and will be ignored. + propertiesToConsider: ['missing', 'esm2015', 'esm5'], + compileAllFormats: false, logger, + }); + + expect(logger.logs.info).toContain([ + 'The target entry-point has already been processed' + ]); + }); + }); + }); + + + function markPropertiesAsProcessed(packagePath: string, properties: EntryPointJsonProperty[]) { + const basePath = '/node_modules'; + const targetPackageJsonPath = _(join(basePath, packagePath, 'package.json')); + const targetPackage = loadPackage(packagePath); + markAsProcessed(targetPackage, targetPackageJsonPath, 'typings'); + properties.forEach(property => markAsProcessed(targetPackage, targetPackageJsonPath, property)); + } + + describe('with propertiesToConsider', () => { it('should only compile the entry-point formats given in the `propertiesToConsider` list', () => {