perf(ivy): ngcc - avoid unnecessary file-write operations when marking properties as processed (#32003)

Previously, when `ngcc` needed to mark multiple properties as processed
(e.g. a processed format property and `typings` or all supported
properties for a non-Angular entry-point), it would update each one
separately and write the file to disk multiple times.

This commit changes this, so that multiple properties can be updated at
once with one file-write operation. While this theoretically improves
performance (reducing the I/O operations), it is not expected to have
any noticeable impact in practice, since these operations are a tiny
fraction of `ngcc`'s work.

This change will be useful for a subsequent change to mark all
properties that map to the same `formatPath` as processed, once it is
processed the first time.

PR Close #32003

packages/compiler-cli/ngcc/src/main.ts

@@ -147,10 +147,11 @@ export function mainNgcc(
       // Either this format was just compiled or its underlying format was compiled because of a
       // previous property.
       if (compiledFormats.has(formatPath)) {
-        markAsProcessed(fileSystem, entryPointPackageJson, entryPointPackageJsonPath, property);
+        const propsToMarkAsProcessed = [property];
-        if (processDts) {
+        if (processDts) propsToMarkAsProcessed.push('typings');
-          markAsProcessed(fileSystem, entryPointPackageJson, entryPointPackageJsonPath, 'typings');
+
-        }
+        markAsProcessed(
+            fileSystem, entryPointPackageJson, entryPointPackageJsonPath, propsToMarkAsProcessed);
       }
     }
 
@@ -193,7 +194,7 @@ function getTargetedEntryPoints(
       fs, config, logger, resolver, basePath, absoluteTargetEntryPointPath, pathMappings);
   const entryPointInfo = finder.findEntryPoints();
   if (entryPointInfo.entryPoints.length === 0) {
-    markNonAngularPackageAsProcessed(fs, absoluteTargetEntryPointPath, propertiesToConsider);
+    markNonAngularPackageAsProcessed(fs, absoluteTargetEntryPointPath);
   }
   return entryPointInfo;
 }
@@ -242,14 +243,13 @@ function hasProcessedTargetEntryPoint(
  * So mark all formats in this entry-point as processed so that clients of ngcc can avoid
  * triggering ngcc for this entry-point in the future.
  */
-function markNonAngularPackageAsProcessed(
+function markNonAngularPackageAsProcessed(fs: FileSystem, path: AbsoluteFsPath) {
-    fs: FileSystem, path: AbsoluteFsPath, propertiesToConsider: string[]) {
   const packageJsonPath = resolve(path, 'package.json');
   const packageJson = JSON.parse(fs.readFile(packageJsonPath));
-  propertiesToConsider.forEach(formatProperty => {
+
-    if (packageJson[formatProperty])
+  // Note: We are marking all supported properties as processed, even if they don't exist in the
-      markAsProcessed(fs, packageJson, packageJsonPath, formatProperty as EntryPointJsonProperty);
+  //       `package.json` file. While this is redundant, it is also harmless.
-  });
+  markAsProcessed(fs, packageJson, packageJsonPath, SUPPORTED_FORMAT_PROPERTIES);
 }
 
 function logInvalidEntryPoints(logger: Logger, invalidEntryPoints: InvalidEntryPoint[]): void {

packages/compiler-cli/ngcc/src/packages/build_marker.ts

@@ -38,17 +38,25 @@ export function hasBeenProcessed(
 }
 
 /**
- * Write a build marker for the given entry-point and format property, to indicate that it has
+ * Write a build marker for the given entry-point and format properties, to indicate that they have
  * been compiled by this version of ngcc.
  *
- * @param entryPoint the entry-point to write a marker.
+ * @param fs The current file-system being used.
- * @param format the property in the package.json of the format for which we are writing the marker.
+ * @param packageJson The parsed contents of the `package.json` file for the entry-point.
+ * @param packageJsonPath The absolute path to the `package.json` file.
+ * @param properties The properties in the `package.json` of the formats for which we are writing
+ *                   the marker.
  */
 export function markAsProcessed(
     fs: FileSystem, packageJson: EntryPointPackageJson, packageJsonPath: AbsoluteFsPath,
-    format: EntryPointJsonProperty) {
+    properties: EntryPointJsonProperty[]) {
-  if (!packageJson.__processed_by_ivy_ngcc__) packageJson.__processed_by_ivy_ngcc__ = {};
+  const processed =
-  packageJson.__processed_by_ivy_ngcc__[format] = NGCC_VERSION;
+      packageJson.__processed_by_ivy_ngcc__ || (packageJson.__processed_by_ivy_ngcc__ = {});
+
+  for (const prop of properties) {
+    processed[prop] = NGCC_VERSION;
+  }
+
   // Just in case this package.json was synthesized due to a custom configuration
   // we will ensure that the path to the containing folder exists before we write the file.
   fs.ensureDir(dirname(packageJsonPath));

packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts

@@ -86,6 +86,12 @@ runInEachFileSystem(() => {
         // `test-package` has no Angular but is marked as processed.
         expect(loadPackage('test-package').__processed_by_ivy_ngcc__).toEqual({
           es2015: '0.0.0-PLACEHOLDER',
+          esm2015: '0.0.0-PLACEHOLDER',
+          esm5: '0.0.0-PLACEHOLDER',
+          fesm2015: '0.0.0-PLACEHOLDER',
+          fesm5: '0.0.0-PLACEHOLDER',
+          main: '0.0.0-PLACEHOLDER',
+          module: '0.0.0-PLACEHOLDER',
         });
 
         // * `core` is a dependency of `test-package`, but it is not processed, since test-package
@@ -164,9 +170,7 @@ runInEachFileSystem(() => {
       const basePath = _('/node_modules');
       const targetPackageJsonPath = join(basePath, packagePath, 'package.json');
       const targetPackage = loadPackage(packagePath);
-      markAsProcessed(fs, targetPackage, targetPackageJsonPath, 'typings');
+      markAsProcessed(fs, targetPackage, targetPackageJsonPath, ['typings', ...properties]);
-      properties.forEach(
-          property => markAsProcessed(fs, targetPackage, targetPackageJsonPath, property));
     }
 
 

packages/compiler-cli/ngcc/test/packages/build_marker_spec.ts

@@ -79,31 +79,54 @@ runInEachFileSystem(() => {
     });
 
     describe('markAsProcessed', () => {
-      it('should write a property in the package.json containing the version placeholder', () => {
+      it('should write properties in the package.json containing the version placeholder', () => {
         const COMMON_PACKAGE_PATH = _('/node_modules/@angular/common/package.json');
         const fs = getFileSystem();
         let pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
         expect(pkg.__processed_by_ivy_ngcc__).toBeUndefined();
-        expect(pkg.__processed_by_ivy_ngcc__).toBeUndefined();
 
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, 'fesm2015');
+        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['fesm2015', 'fesm5']);
         pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
-        expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toEqual('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toBe('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.fesm5).toBe('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.esm2015).toBeUndefined();
         expect(pkg.__processed_by_ivy_ngcc__.esm5).toBeUndefined();
 
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, 'esm5');
+        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['esm2015', 'esm5']);
         pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
-        expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toEqual('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toBe('0.0.0-PLACEHOLDER');
-        expect(pkg.__processed_by_ivy_ngcc__.esm5).toEqual('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.fesm5).toBe('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.esm2015).toBe('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.esm5).toBe('0.0.0-PLACEHOLDER');
       });
 
       it('should update the packageJson object in-place', () => {
         const COMMON_PACKAGE_PATH = _('/node_modules/@angular/common/package.json');
         const fs = getFileSystem();
-        let pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
+        const pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
         expect(pkg.__processed_by_ivy_ngcc__).toBeUndefined();
-        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, 'fesm2015');
+
-        expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toEqual('0.0.0-PLACEHOLDER');
+        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['fesm2015', 'fesm5']);
+        expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toBe('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.fesm5).toBe('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.esm2015).toBeUndefined();
+        expect(pkg.__processed_by_ivy_ngcc__.esm5).toBeUndefined();
+
+        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['esm2015', 'esm5']);
+        expect(pkg.__processed_by_ivy_ngcc__.fesm2015).toBe('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.fesm5).toBe('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.esm2015).toBe('0.0.0-PLACEHOLDER');
+        expect(pkg.__processed_by_ivy_ngcc__.esm5).toBe('0.0.0-PLACEHOLDER');
+      });
+
+      it('should one perform one write operation for all updated properties', () => {
+        const COMMON_PACKAGE_PATH = _('/node_modules/@angular/common/package.json');
+        const fs = getFileSystem();
+        const writeFileSpy = spyOn(fs, 'writeFile');
+        let pkg = JSON.parse(fs.readFile(COMMON_PACKAGE_PATH));
+
+        markAsProcessed(fs, pkg, COMMON_PACKAGE_PATH, ['fesm2015', 'fesm5', 'esm2015', 'esm5']);
+        expect(writeFileSpy).toHaveBeenCalledTimes(1);
       });
     });