fix(ngcc): do not spawn unlocker processes on cluster workers (#36569)
The current ngcc lock-file strategy spawns a new process in order to capture a potential `SIGINT` and remove the lock-file. For more information see #35861. Previously, this unlocker process was spawned as soon as the `LockFile` was instantiated in order to have it available as soon as possible (given that spawning a process is an asynchronous operation). Since the `LockFile` was instantiated and passed to the `Executor`, this meant that an unlocker process was spawned for each cluster worker, when running ngcc in parallel mode. These processes were not needed, since the `LockFile` was not used in cluster workers, but we still had to pay the overhead of each process' own memory and V8 instance. (NOTE: This overhead was small compared to the memory consumed by ngcc's normal operations, but still unnecessary.) This commit avoids the extra processes by only spawning an unlocker process when running on the cluster master process and not on worker processes. PR Close #36569
This commit is contained in:
parent
663b768780
commit
66effde9f3
|
@ -0,0 +1,42 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright Google Inc. All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
|
||||
/// <reference types="node" />
|
||||
|
||||
import {ChildProcess} from 'child_process';
|
||||
import * as cluster from 'cluster';
|
||||
|
||||
import {AbsoluteFsPath} from '../../../../src/ngtsc/file_system';
|
||||
import {LockFileWithChildProcess} from '../../locking/lock_file_with_child_process';
|
||||
|
||||
|
||||
/**
|
||||
* A `LockFileWithChildProcess` that is `cluster`-aware and does not spawn unlocker processes from
|
||||
* worker processes (only from the master process, which does the locking).
|
||||
*/
|
||||
export class ClusterLockFileWithChildProcess extends LockFileWithChildProcess {
|
||||
write(): void {
|
||||
if (!cluster.isMaster) {
|
||||
// This is a worker process:
|
||||
// This method should only be on the master process.
|
||||
throw new Error('Tried to create a lock-file from a worker process.');
|
||||
}
|
||||
|
||||
return super.write();
|
||||
}
|
||||
|
||||
protected createUnlocker(path: AbsoluteFsPath): ChildProcess|null {
|
||||
if (cluster.isMaster) {
|
||||
// This is the master process:
|
||||
// Create the unlocker.
|
||||
return super.createUnlocker(path);
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
}
|
|
@ -77,7 +77,7 @@ export class LockFileWithChildProcess implements LockFile {
|
|||
}
|
||||
}
|
||||
|
||||
protected createUnlocker(path: AbsoluteFsPath): ChildProcess {
|
||||
protected createUnlocker(path: AbsoluteFsPath): ChildProcess|null {
|
||||
this.logger.debug('Forking unlocker child-process');
|
||||
const logLevel =
|
||||
this.logger.level !== undefined ? this.logger.level.toString() : LogLevel.info.toString();
|
||||
|
|
|
@ -27,6 +27,7 @@ import {EntryPointFinder} from './entry_point_finder/interface';
|
|||
import {TargetedEntryPointFinder} from './entry_point_finder/targeted_entry_point_finder';
|
||||
import {AnalyzeEntryPointsFn, CreateCompileFn, Executor} from './execution/api';
|
||||
import {ClusterExecutor} from './execution/cluster/executor';
|
||||
import {ClusterLockFileWithChildProcess} from './execution/cluster/lock_file_with_child_process';
|
||||
import {ClusterPackageJsonUpdater} from './execution/cluster/package_json_updater';
|
||||
import {SingleProcessExecutorAsync, SingleProcessExecutorSync} from './execution/single_process_executor';
|
||||
import {CreateTaskCompletedCallback, PartiallyOrderedTasks, Task, TaskProcessingOutcome, TaskQueue} from './execution/tasks/api';
|
||||
|
@ -428,7 +429,8 @@ function getCreateTaskCompletedCallback(
|
|||
function getExecutor(
|
||||
async: boolean, inParallel: boolean, logger: Logger, pkgJsonUpdater: PackageJsonUpdater,
|
||||
fileSystem: FileSystem, createTaskCompletedCallback: CreateTaskCompletedCallback): Executor {
|
||||
const lockFile = new LockFileWithChildProcess(fileSystem, logger);
|
||||
const lockFile = inParallel ? new ClusterLockFileWithChildProcess(fileSystem, logger) :
|
||||
new LockFileWithChildProcess(fileSystem, logger);
|
||||
if (async) {
|
||||
// Execute asynchronously (either serially or in parallel)
|
||||
const locker = new AsyncLocker(lockFile, logger, 500, 50);
|
||||
|
|
|
@ -0,0 +1,87 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright Google Inc. All Rights Reserved.
|
||||
*
|
||||
* Use of this source code is governed by an MIT-style license that can be
|
||||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
|
||||
/// <reference types="node" />
|
||||
|
||||
import {ChildProcess} from 'child_process';
|
||||
import * as cluster from 'cluster';
|
||||
|
||||
import {getFileSystem} from '../../../../src/ngtsc/file_system';
|
||||
import {runInEachFileSystem} from '../../../../src/ngtsc/file_system/testing';
|
||||
import {ClusterLockFileWithChildProcess} from '../../../src/execution/cluster/lock_file_with_child_process';
|
||||
import {LockFileWithChildProcess} from '../../../src/locking/lock_file_with_child_process';
|
||||
import {MockLogger} from '../../helpers/mock_logger';
|
||||
import {mockProperty} from '../../helpers/spy_utils';
|
||||
|
||||
|
||||
runInEachFileSystem(() => {
|
||||
describe('ClusterLockFileWithChildProcess', () => {
|
||||
const runAsClusterMaster = mockProperty(cluster, 'isMaster');
|
||||
const mockUnlockerProcess = {} as ChildProcess;
|
||||
let lockFileWithChildProcessSpies:
|
||||
Record<'createUnlocker'|'read'|'remove'|'write', jasmine.Spy>;
|
||||
|
||||
beforeEach(() => {
|
||||
lockFileWithChildProcessSpies = {
|
||||
createUnlocker: spyOn(LockFileWithChildProcess.prototype as any, 'createUnlocker')
|
||||
.and.returnValue(mockUnlockerProcess),
|
||||
read: spyOn(LockFileWithChildProcess.prototype, 'read').and.returnValue('{unknown}'),
|
||||
remove: spyOn(LockFileWithChildProcess.prototype, 'remove'),
|
||||
write: spyOn(LockFileWithChildProcess.prototype, 'write'),
|
||||
};
|
||||
});
|
||||
|
||||
it('should be an instance of `LockFileWithChildProcess`', () => {
|
||||
const lockFile = new ClusterLockFileWithChildProcess(getFileSystem(), new MockLogger());
|
||||
|
||||
expect(lockFile).toEqual(jasmine.any(ClusterLockFileWithChildProcess));
|
||||
expect(lockFile).toEqual(jasmine.any(LockFileWithChildProcess));
|
||||
});
|
||||
|
||||
describe('write()', () => {
|
||||
it('should create the lock-file when called on the cluster master', () => {
|
||||
runAsClusterMaster(true);
|
||||
const lockFile = new ClusterLockFileWithChildProcess(getFileSystem(), new MockLogger());
|
||||
|
||||
expect(lockFileWithChildProcessSpies.write).not.toHaveBeenCalled();
|
||||
|
||||
lockFile.write();
|
||||
expect(lockFileWithChildProcessSpies.write).toHaveBeenCalledWith();
|
||||
});
|
||||
|
||||
it('should throw an error when called on a cluster worker', () => {
|
||||
runAsClusterMaster(false);
|
||||
const lockFile = new ClusterLockFileWithChildProcess(getFileSystem(), new MockLogger());
|
||||
|
||||
expect(() => lockFile.write())
|
||||
.toThrowError('Tried to create a lock-file from a worker process.');
|
||||
expect(lockFileWithChildProcessSpies.write).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('createUnlocker()', () => {
|
||||
it('should create the unlocker when called on the cluster master', () => {
|
||||
runAsClusterMaster(true);
|
||||
const lockFile = new ClusterLockFileWithChildProcess(getFileSystem(), new MockLogger());
|
||||
|
||||
lockFileWithChildProcessSpies.createUnlocker.calls.reset();
|
||||
|
||||
expect((lockFile as any).createUnlocker(lockFile.path)).toBe(mockUnlockerProcess);
|
||||
expect(lockFileWithChildProcessSpies.createUnlocker).toHaveBeenCalledWith(lockFile.path);
|
||||
});
|
||||
|
||||
it('should not create the unlocker when called on a cluster worker', () => {
|
||||
runAsClusterMaster(false);
|
||||
const lockFile = new ClusterLockFileWithChildProcess(getFileSystem(), new MockLogger());
|
||||
|
||||
expect((lockFile as any).createUnlocker(lockFile.path)).toBeNull();
|
||||
expect(lockFileWithChildProcessSpies.createUnlocker).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
Loading…
Reference in New Issue