From 0d1a4660e4d440b5de157bf933e21af684a10c16 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Wed, 20 Dec 2017 01:28:16 +0200 Subject: [PATCH] Fix EMFILE error when running on Android During running on Android, CLI tries to take smart decision which files to upload. This is done by checking the shasum of all project files and comparing it with the latest one uploaded to device. However, in huge projects, this leads to error with code EMFILE as the OS have limits for opened file handles. This happens as CLI executes the operation simultaneously for all files. In order to fix this behavior, execute the actions by chunks (currently set to 100). Introduce new method in helpers to execute this action and add tests for it. --- constants.ts | 2 + definitions/mobile.d.ts | 12 ++- helpers.ts | 18 +++++ mobile/android/android-device-file-system.ts | 78 ++++++++----------- mobile/android/android-device-hash-service.ts | 51 ++++++------ test/unit-tests/helpers.ts | 55 +++++++++++++ 6 files changed, 139 insertions(+), 77 deletions(-) diff --git a/constants.ts b/constants.ts index 2020c293..1468be99 100644 --- a/constants.ts +++ b/constants.ts @@ -91,3 +91,5 @@ export const enum AnalyticsClients { NonInteractive = "Non-interactive", Unknown = "Unknown" } + +export const DEFAULT_CHUNK_SIZE = 100; diff --git a/definitions/mobile.d.ts b/definitions/mobile.d.ts index 4e141953..92076be7 100644 --- a/definitions/mobile.d.ts +++ b/definitions/mobile.d.ts @@ -671,9 +671,9 @@ declare module Mobile { */ getShasumsFromDevice(): Promise; /** - * Computes the shasums of localToDevicePaths and changes the content of hash file on device + * Uploads updated shasums to hash file on device */ - uploadHashFileToDevice(data: IStringDictionary | Mobile.ILocalToDevicePathData[]): Promise; + uploadHashFileToDevice(data: IStringDictionary): Promise; /** * Computes the shasums of localToDevicePaths and updates hash file on device */ @@ -688,6 +688,14 @@ declare module Mobile { * @return {Promise} boolean True if file exists and false otherwise. */ doesShasumFileExistsOnDevice(): Promise; + + /** + * Generates hashes of specified localToDevicePaths by chunks and persists them in the passed @shasums argument. + * @param {Mobile.ILocalToDevicePathData[]} localToDevicePaths The localToDevicePaths objects for which the hashes should be generated. + * @param {IStringDicitionary} shasums Object in which the shasums will be persisted. + * @returns {Promise[]} DevicePaths of all elements from the input localToDevicePaths. + */ + generateHashesFromLocalToDevicePaths(localToDevicePaths: Mobile.ILocalToDevicePathData[], shasums: IStringDictionary): Promise; } /** diff --git a/helpers.ts b/helpers.ts index dd9b7ba0..a0fe50ca 100644 --- a/helpers.ts +++ b/helpers.ts @@ -8,6 +8,24 @@ import * as crypto from "crypto"; const Table = require("cli-table"); +export async function executeActionByChunks(initialData: T[] | IDictionary, chunkSize: number, elementAction: (element: T, key?: string | number) => Promise): Promise { + let arrayToChunk: (T | string)[]; + let action: (key: string | T) => Promise; + + if (_.isArray(initialData)) { + arrayToChunk = initialData; + action = (element: T) => elementAction(element, initialData.indexOf(element)); + } else { + arrayToChunk = _.keys(initialData); + action = (key: string) => elementAction(initialData[key], key); + } + + const chunks = _.chunk(arrayToChunk, chunkSize); + for (const chunk of chunks) { + await Promise.all(_.map(chunk, element => action(element))); + } +} + export function deferPromise(): IDeferPromise { let resolve: (value?: T | PromiseLike) => void; let reject: (reason?: any) => void; diff --git a/mobile/android/android-device-file-system.ts b/mobile/android/android-device-file-system.ts index bd9cb4aa..72fecbce 100644 --- a/mobile/android/android-device-file-system.ts +++ b/mobile/android/android-device-file-system.ts @@ -1,6 +1,8 @@ import * as path from "path"; import * as temp from "temp"; import { AndroidDeviceHashService } from "./android-device-hash-service"; +import { executeActionByChunks } from "../../helpers"; +import { DEFAULT_CHUNK_SIZE } from '../../constants'; export class AndroidDeviceFileSystem implements Mobile.IDeviceFileSystem { private _deviceHashServices = Object.create(null); @@ -51,28 +53,24 @@ export class AndroidDeviceFileSystem implements Mobile.IDeviceFileSystem { } public async transferFiles(deviceAppData: Mobile.IDeviceAppData, localToDevicePaths: Mobile.ILocalToDevicePathData[]): Promise { - // TODO: Do not start all promises simultaneously as this leads to error EMFILE on Windows for too many opened files. - // Use chunks (for example on 100). - await Promise.all( - _(localToDevicePaths) - .filter(localToDevicePathData => this.$fs.getFsStats(localToDevicePathData.getLocalPath()).isFile()) - .map(async localToDevicePathData => { - const devicePath = localToDevicePathData.getDevicePath(); - await this.adb.executeCommand(["push", localToDevicePathData.getLocalPath(), devicePath]); - await this.adb.executeShellCommand(["chmod", "0777", path.dirname(devicePath)]); - } - ) - .value() - ); - - await Promise.all( - _(localToDevicePaths) - .filter(localToDevicePathData => this.$fs.getFsStats(localToDevicePathData.getLocalPath()).isDirectory()) - .map(async localToDevicePathData => - await this.adb.executeShellCommand(["chmod", "0777", localToDevicePathData.getDevicePath()]) - ) - .value() - ); + const directoriesToChmod: string[] = []; + const action = async (localToDevicePathData: Mobile.ILocalToDevicePathData) => { + const fstat = this.$fs.getFsStats(localToDevicePathData.getLocalPath()); + if (fstat.isFile()) { + const devicePath = localToDevicePathData.getDevicePath(); + await this.adb.executeCommand(["push", localToDevicePathData.getLocalPath(), devicePath]); + await this.adb.executeShellCommand(["chmod", "0777", path.dirname(devicePath)]); + } else if (fstat.isDirectory()) { + const dirToChmod = localToDevicePathData.getDevicePath(); + directoriesToChmod.push(dirToChmod); + } + }; + + await executeActionByChunks(localToDevicePaths, DEFAULT_CHUNK_SIZE, action); + + const dirsChmodAction = (directoryToChmod: string) => this.adb.executeShellCommand(["chmod", "0777", directoryToChmod]); + + await executeActionByChunks(_.uniq(directoriesToChmod), DEFAULT_CHUNK_SIZE, dirsChmodAction); // Update hashes const deviceHashService = this.getDeviceHashService(deviceAppData.appIdentifier); @@ -82,25 +80,12 @@ export class AndroidDeviceFileSystem implements Mobile.IDeviceFileSystem { } public async transferDirectory(deviceAppData: Mobile.IDeviceAppData, localToDevicePaths: Mobile.ILocalToDevicePathData[], projectFilesPath: string): Promise { - const devicePaths: string[] = []; const currentShasums: IStringDictionary = {}; - - await Promise.all( - localToDevicePaths.map(async localToDevicePathData => { - const localPath = localToDevicePathData.getLocalPath(); - const stats = this.$fs.getFsStats(localPath); - if (stats.isFile()) { - const fileShasum = await this.$fs.getFileShasum(localPath); - currentShasums[localPath] = fileShasum; - } - - devicePaths.push(`"${localToDevicePathData.getDevicePath()}"`); - }) - ); + const deviceHashService = this.getDeviceHashService(deviceAppData.appIdentifier); + const devicePaths: string[] = await deviceHashService.generateHashesFromLocalToDevicePaths(localToDevicePaths, currentShasums); const commandsDeviceFilePath = this.$mobileHelper.buildDevicePath(await deviceAppData.getDeviceProjectRootPath(), "nativescript.commands.sh"); - const deviceHashService = this.getDeviceHashService(deviceAppData.appIdentifier); let filesToChmodOnDevice: string[] = devicePaths; let tranferredFiles: Mobile.ILocalToDevicePathData[] = []; const oldShasums = await deviceHashService.getShasumsFromDevice(); @@ -113,16 +98,15 @@ export class AndroidDeviceFileSystem implements Mobile.IDeviceFileSystem { const changedShasums: any = _.omitBy(currentShasums, (hash: string, pathToFile: string) => !!_.find(oldShasums, (oldHash: string, oldPath: string) => pathToFile === oldPath && hash === oldHash)); this.$logger.trace("Changed file hashes are:", changedShasums); filesToChmodOnDevice = []; - await Promise.all( - _(changedShasums) - .map((hash: string, filePath: string) => _.find(localToDevicePaths, ldp => ldp.getLocalPath() === filePath)) - .map(localToDevicePathData => { - tranferredFiles.push(localToDevicePathData); - filesToChmodOnDevice.push(`"${localToDevicePathData.getDevicePath()}"`); - return this.transferFile(localToDevicePathData.getLocalPath(), localToDevicePathData.getDevicePath()); - }) - .value() - ); + + const transferFileAction = async (hash: string, filePath: string) => { + const localToDevicePathData = _.find(localToDevicePaths, ldp => ldp.getLocalPath() === filePath); + tranferredFiles.push(localToDevicePathData); + filesToChmodOnDevice.push(`"${localToDevicePathData.getDevicePath()}"`); + return this.transferFile(localToDevicePathData.getLocalPath(), localToDevicePathData.getDevicePath()); + }; + + await executeActionByChunks(changedShasums, DEFAULT_CHUNK_SIZE, transferFileAction); } if (filesToChmodOnDevice.length) { diff --git a/mobile/android/android-device-hash-service.ts b/mobile/android/android-device-hash-service.ts index 0e4a7b44..800debb4 100644 --- a/mobile/android/android-device-hash-service.ts +++ b/mobile/android/android-device-hash-service.ts @@ -1,6 +1,8 @@ import * as path from "path"; import * as temp from "temp"; import { cache } from "../../decorators"; +import { executeActionByChunks } from "../../helpers"; +import { DEFAULT_CHUNK_SIZE } from "../../constants"; export class AndroidDeviceHashService implements Mobile.IAndroidDeviceHashService { private static HASH_FILE_NAME = "hashes"; @@ -32,40 +34,15 @@ export class AndroidDeviceHashService implements Mobile.IAndroidDeviceHashServic return null; } - public async uploadHashFileToDevice(data: IStringDictionary | Mobile.ILocalToDevicePathData[]): Promise { - let shasums: IStringDictionary = {}; - if (_.isArray(data)) { - await Promise.all( - (data).map(async localToDevicePathData => { - const localPath = localToDevicePathData.getLocalPath(); - const stats = this.$fs.getFsStats(localPath); - if (stats.isFile()) { - const fileShasum = await this.$fs.getFileShasum(localPath); - shasums[localPath] = fileShasum; - } - }) - ); - } else { - shasums = data; - } - - this.$fs.writeJson(this.hashFileLocalPath, shasums); + public async uploadHashFileToDevice(data: IStringDictionary): Promise { + this.$fs.writeJson(this.hashFileLocalPath, data); await this.adb.executeCommand(["push", this.hashFileLocalPath, this.hashFileDevicePath]); } public async updateHashes(localToDevicePaths: Mobile.ILocalToDevicePathData[]): Promise { const oldShasums = await this.getShasumsFromDevice(); if (oldShasums) { - await Promise.all( - _.map(localToDevicePaths, async ldp => { - const localPath = ldp.getLocalPath(); - if (this.$fs.getFsStats(localPath).isFile()) { - // TODO: Use relative to project path for key - // This will speed up livesync on the same device for the same project on different PCs. - oldShasums[localPath] = await this.$fs.getFileShasum(localPath); - } - }) - ); + await this.generateHashesFromLocalToDevicePaths(localToDevicePaths, oldShasums); await this.uploadHashFileToDevice(oldShasums); @@ -75,6 +52,24 @@ export class AndroidDeviceHashService implements Mobile.IAndroidDeviceHashServic return false; } + public async generateHashesFromLocalToDevicePaths(localToDevicePaths: Mobile.ILocalToDevicePathData[], shasums: IStringDictionary): Promise { + const devicePaths: string[] = []; + const action = async (localToDevicePathData: Mobile.ILocalToDevicePathData) => { + const localPath = localToDevicePathData.getLocalPath(); + if (this.$fs.getFsStats(localPath).isFile()) { + // TODO: Use relative to project path for key + // This will speed up livesync on the same device for the same project on different PCs. + shasums[localPath] = await this.$fs.getFileShasum(localPath); + } + + devicePaths.push(`"${localToDevicePathData.getDevicePath()}"`); + }; + + await executeActionByChunks(localToDevicePaths, DEFAULT_CHUNK_SIZE, action); + + return devicePaths; + } + public async removeHashes(localToDevicePaths: Mobile.ILocalToDevicePathData[]): Promise { const oldShasums = await this.getShasumsFromDevice(); if (oldShasums) { diff --git a/test/unit-tests/helpers.ts b/test/unit-tests/helpers.ts index f79164c5..3b1f3e9e 100644 --- a/test/unit-tests/helpers.ts +++ b/test/unit-tests/helpers.ts @@ -15,6 +15,61 @@ describe("helpers", () => { assert.deepEqual(actualResult, testData.expectedResult, `For input ${testData.input}, the expected result is: ${testData.expectedResult}, but actual result is: ${actualResult}.`); }; + describe("executeActionByChunks", () => { + const chunkSize = 2; + + const assertElements = (initialDataValues: any[], handledElements: any[], element: any, passedChunkSize: number) => { + return new Promise(resolve => setImmediate(() => { + const remainingElements = _.difference(initialDataValues, handledElements); + const isFromLastChunk = (element + passedChunkSize) > initialDataValues.length; + // If the element is one of the last chunk, the remainingElements must be empty. + // If the element is from any other chunk, the remainingElements must contain all elements outside of this chunk. + if (isFromLastChunk) { + assert.isTrue(!remainingElements.length); + } else { + const indexOfElement = initialDataValues.indexOf(element); + const chunkNumber = Math.floor(indexOfElement / passedChunkSize) + 1; + const expectedRemainingElements = _.drop(initialDataValues, chunkNumber * passedChunkSize); + + assert.deepEqual(remainingElements, expectedRemainingElements); + } + + resolve(); + })); + }; + + it("works correctly with array", () => { + const initialData = _.range(7); + const handledElements: number[] = []; + + return helpers.executeActionByChunks(initialData, chunkSize, (element: number, index: number) => { + handledElements.push(element); + assert.deepEqual(element, initialData[index]); + assert.isTrue(initialData.indexOf(element) !== -1); + return assertElements(initialData, handledElements, element, chunkSize); + }); + }); + + it("works correctly with IDictionary", () => { + const initialData: IDictionary = { + a: 1, + b: 2, + c: 3, + d: 4, + e: 5, + f: 6 + }; + + const initialDataValues = _.values(initialData); + const handledElements: number[] = []; + return helpers.executeActionByChunks(initialData, chunkSize, (element, key) => { + handledElements.push(element); + assert.isTrue(initialData[key] === element); + return assertElements(initialDataValues, handledElements, element, chunkSize); + }); + }); + }); + describe("getPropertyName", () => { const ES5Functions: ITestData[] = [ {