fix(service-worker): file system hash in batch of 500 elements (#45262)

Luca · AndrewKushnir · commit cff1c5622f66 · 2022-03-08T10:23:39.000-08:00
Add file system concurrency hash test Fixes #45133 PR Close #45262
diff --git a/packages/service-worker/config/src/generator.ts b/packages/service-worker/config/src/generator.ts
@@ -67,7 +67,8 @@ export class Generator {
 
     // Compute hashes for all matched files and add them to the hash-table.
     const allMatchedFiles = ([] as string[]).concat(...Array.from(filesPerGroup.values())).sort();
-    const allMatchedHashes = await Promise.all(allMatchedFiles.map(file => this.fs.hash(file)));
+    const allMatchedHashes =
+        await processInBatches(allMatchedFiles, 500, file => this.fs.hash(file));
     allMatchedFiles.forEach((file, idx) => {
       hashTable[joinUrls(this.baseHref, file)] = allMatchedHashes[idx];
     });
@@ -110,6 +111,20 @@ export function processNavigationUrls(
   });
 }
 
+async function processInBatches<I, O>(
+    items: I[], batchSize: number, processFn: (item: I) => O | Promise<O>): Promise<O[]> {
+  const batches = [];
+
+  for (let i = 0; i < items.length; i += batchSize) {
+    batches.push(items.slice(i, i + batchSize));
+  }
+
+  return batches.reduce(
+      async (prev, batch) =>
+          (await prev).concat(await Promise.all(batch.map(item => processFn(item)))),
+      Promise.resolve<O[]>([]));
+}
+
 function globListToMatcher(globs: string[]): (file: string) => boolean {
   const patterns = globs.map(pattern => {
     if (pattern.startsWith('!')) {
diff --git a/packages/service-worker/config/test/generator_spec.ts b/packages/service-worker/config/test/generator_spec.ts
@@ -8,7 +8,7 @@
 
 import {Generator, processNavigationUrls} from '../src/generator';
 import {AssetGroup} from '../src/in';
-import {MockFilesystem} from '../testing/mock';
+import {HashTrackingMockFilesystem, MockFilesystem} from '../testing/mock';
 
 describe('Generator', () => {
   beforeEach(() => spyOn(Date, 'now').and.returnValue(1234567890123));
@@ -355,6 +355,26 @@ describe('Generator', () => {
     });
   });
 
+  it('doesn\'t exceed concurrency limit', async () => {
+    const fileCount = 600;
+    const files = [...Array(fileCount).keys()].reduce((acc: Record<string, string>, _, i) => {
+      acc[`/test${i}.js`] = `This is a test ${i}`;
+      return acc;
+    }, {'/index.html': 'This is a test'});
+    const fs = new HashTrackingMockFilesystem(files);
+    const gen = new Generator(fs, '/');
+    const config = await gen.process({
+      index: '/index.html',
+      assetGroups: [{
+        name: 'test',
+        resources: {files: ['/*.js']},
+      }],
+    });
+    expect(fs.maxConcurrentHashings).toBeLessThanOrEqual(500);
+    expect(fs.maxConcurrentHashings).toBeGreaterThan(1);
+    expect(Object.keys((config as any).hashTable).length).toBe(fileCount);
+  });
+
   describe('processNavigationUrls()', () => {
     const customNavigationUrls = [
       'https://host/positive/external/**',
diff --git a/packages/service-worker/config/testing/mock.ts b/packages/service-worker/config/testing/mock.ts
@@ -32,3 +32,23 @@ export class MockFilesystem implements Filesystem {
     this.files.set(path, contents);
   }
 }
+
+export class HashTrackingMockFilesystem extends MockFilesystem {
+  public maxConcurrentHashings = 0;
+  private concurrentHashings = 0;
+
+  /** @override */
+  override async hash(path: string): Promise<string> {
+    // Increase the concurrent hashings count.
+    this.concurrentHashings += 1;
+    this.maxConcurrentHashings = Math.max(this.maxConcurrentHashings, this.concurrentHashings);
+
+    // Artificial delay to check hashing concurrency.
+    await new Promise(resolve => setTimeout(resolve, 250));
+
+    // Decrease the concurrent hashings count.
+    this.concurrentHashings -= 1;
+
+    return super.hash(path);
+  }
+}
