Skip to content

Commit

Permalink
fix(service-worker): file system hash in batch of 500 elements (#45262)
Browse files Browse the repository at this point in the history
Add file system concurrency hash test

Fixes #45133
PR Close #45262
  • Loading branch information
Luca authored and AndrewKushnir committed Mar 8, 2022
1 parent 6efa366 commit cff1c56
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 2 deletions.
17 changes: 16 additions & 1 deletion packages/service-worker/config/src/generator.ts
Expand Up @@ -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];
});
Expand Down Expand Up @@ -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('!')) {
Expand Down
22 changes: 21 additions & 1 deletion packages/service-worker/config/test/generator_spec.ts
Expand Up @@ -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));
Expand Down Expand Up @@ -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/**',
Expand Down
20 changes: 20 additions & 0 deletions packages/service-worker/config/testing/mock.ts
Expand Up @@ -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);
}
}

0 comments on commit cff1c56

Please sign in to comment.