From d423afd75d624b71f30ac4ef8ee205ce2640e1b4 Mon Sep 17 00:00:00 2001 From: John Knox Date: Thu, 13 Aug 2020 03:46:36 -0700 Subject: [PATCH] Fix high pkd CPU usage issue Summary: Running `instruments -s devices` causes the `pkd` process in catalina to spike to ~100% for a few seconds. Flipper runs this command every 3 seconds to poll for devices. This switches it to use `idb list-targets` instead which is much more performant. Currently switched off in the open-source version while we make sure it's working well. If you set the GK value 'flipper_use_idb_to_list_devices' to true, then you'll get the new behaviour. Reviewed By: passy Differential Revision: D23102067 fbshipit-source-id: 9e17155d938a4fe326e082511f747444e4b533a2 --- desktop/app/package.json | 6 +- desktop/app/src/dispatcher/iOSDevice.tsx | 20 ++++--- desktop/app/src/utils/CertificateProvider.tsx | 42 ++++++------- desktop/app/src/utils/iOSContainerUtility.tsx | 60 +++++++++++++------ desktop/app/src/utils/listDevices.tsx | 2 +- desktop/yarn.lock | 7 +++ 6 files changed, 88 insertions(+), 49 deletions(-) diff --git a/desktop/app/package.json b/desktop/app/package.json index a320a0774..6ed10c00c 100644 --- a/desktop/app/package.json +++ b/desktop/app/package.json @@ -24,14 +24,15 @@ "emotion": "^10.0.23", "expand-tilde": "^2.0.2", "flipper-client-sdk": "^0.0.2", - "flipper-plugin": "0.52.1", "flipper-doctor": "0.52.1", + "flipper-plugin": "0.52.1", "flipper-plugin-lib": "0.52.1", "fs-extra": "^8.0.1", "immer": "^6.0.0", "immutable": "^4.0.0-rc.12", "invariant": "^2.2.2", "lodash": "^4.17.19", + "lodash.memoize": "^4.1.2", "npm-api": "^1.0.0", "open": "^7.0.0", "openssl-wrapper": "^0.3.4", @@ -69,8 +70,9 @@ "7zip-bin-mac": "^1.0.1" }, "devDependencies": { - "@testing-library/react": "^10.4.3", "@testing-library/dom": "^7.20.2", + "@testing-library/react": "^10.4.3", + "@types/lodash.memoize": "^4.1.6", "flipper-test-utils": "0.52.1", "metro": "^0.60.0", "mock-fs": "^4.12.0", diff --git a/desktop/app/src/dispatcher/iOSDevice.tsx b/desktop/app/src/dispatcher/iOSDevice.tsx index 3ba3cf625..03dfbda08 100644 --- a/desktop/app/src/dispatcher/iOSDevice.tsx +++ b/desktop/app/src/dispatcher/iOSDevice.tsx @@ -81,9 +81,11 @@ async function queryDevices(store: Store, logger: Logger): Promise { getActiveSimulators().then((devices) => { processDevices(store, logger, devices, 'emulator'); }), - getActiveDevices().then((devices) => { - processDevices(store, logger, devices, 'physical'); - }), + getActiveDevices(store.getState().settingsState.idbPath).then( + (devices: IOSDeviceParams[]) => { + processDevices(store, logger, devices, 'physical'); + }, + ), ]); } @@ -173,8 +175,8 @@ function getActiveSimulators(): Promise> { .catch((_) => []); } -function getActiveDevices(): Promise> { - return iosUtil.targets().catch((e) => { +function getActiveDevices(idbPath: string): Promise> { + return iosUtil.targets(idbPath).catch((e) => { console.error(e.message); return []; }); @@ -233,12 +235,12 @@ async function isXcodeDetected(): Promise { .catch((_) => false); } -export async function getActiveDevicesAndSimulators(): Promise< - Array -> { +export async function getActiveDevicesAndSimulators( + store: Store, +): Promise> { const activeDevices: Array> = await Promise.all([ getActiveSimulators(), - getActiveDevices(), + getActiveDevices(store.getState().settingsState.idbPath), ]); const allDevices = activeDevices[0].concat(activeDevices[1]); return allDevices.map((device) => { diff --git a/desktop/app/src/utils/CertificateProvider.tsx b/desktop/app/src/utils/CertificateProvider.tsx index dfa0bb31c..731f43748 100644 --- a/desktop/app/src/utils/CertificateProvider.tsx +++ b/desktop/app/src/utils/CertificateProvider.tsx @@ -342,28 +342,30 @@ export default class CertificateProvider { // It's a simulator, the deviceId is in the filepath. return Promise.resolve(matches[1]); } - return iosUtil.targets().then((targets) => { - if (targets.length === 0) { - throw new Error('No iOS devices found'); - } - const deviceMatchList = targets.map((target) => - this.iOSDeviceHasMatchingCSR( - deviceCsrFilePath, - target.udid, - appName, - csr, - ).then((isMatch) => { - return {id: target.udid, isMatch}; - }), - ); - return Promise.all(deviceMatchList).then((devices) => { - const matchingIds = devices.filter((m) => m.isMatch).map((m) => m.id); - if (matchingIds.length == 0) { - throw new Error(`No matching device found for app: ${appName}`); + return iosUtil + .targets(this.store.getState().settingsState.idbPath) + .then((targets) => { + if (targets.length === 0) { + throw new Error('No iOS devices found'); } - return matchingIds[0]; + const deviceMatchList = targets.map((target) => + this.iOSDeviceHasMatchingCSR( + deviceCsrFilePath, + target.udid, + appName, + csr, + ).then((isMatch) => { + return {id: target.udid, isMatch}; + }), + ); + return Promise.all(deviceMatchList).then((devices) => { + const matchingIds = devices.filter((m) => m.isMatch).map((m) => m.id); + if (matchingIds.length == 0) { + throw new Error(`No matching device found for app: ${appName}`); + } + return matchingIds[0]; + }); }); - }); } androidDeviceHasMatchingCSR( diff --git a/desktop/app/src/utils/iOSContainerUtility.tsx b/desktop/app/src/utils/iOSContainerUtility.tsx index dde1df711..65ae3a44f 100644 --- a/desktop/app/src/utils/iOSContainerUtility.tsx +++ b/desktop/app/src/utils/iOSContainerUtility.tsx @@ -10,11 +10,13 @@ import child_process from 'child_process'; import {promisify} from 'util'; import {Mutex} from 'async-mutex'; -import {notNull} from './typeUtils'; const unsafeExec = promisify(child_process.exec); import {killOrphanedInstrumentsProcesses} from './processCleanup'; import {reportPlatformFailures} from './metrics'; import {promises, constants} from 'fs'; +import memoize from 'lodash.memoize'; +import GK from '../fb-stubs/GK'; +import {notNull} from './typeUtils'; // Use debug to get helpful logs when idb fails const idbLogLevel = 'DEBUG'; @@ -22,6 +24,15 @@ const operationPrefix = 'iosContainerUtility'; const mutex = new Mutex(); +type IdbTarget = { + name: string; + udid: string; + state: string; + type: string; + os_version: string; + architecture: string; +}; + export type DeviceTarget = { udid: string; type: 'physical' | 'emulator'; @@ -44,23 +55,38 @@ function safeExec(command: string): Promise<{stdout: string; stderr: string}> { }); } -async function targets(): Promise> { +async function targets(idbPath: string): Promise> { if (process.platform !== 'darwin') { return []; } - await killOrphanedInstrumentsProcesses(); - return safeExec('instruments -s devices').then(({stdout}) => - stdout - .toString() - .split('\n') - .map((line) => line.trim()) - .map((line) => /(.+) \([^(]+\) \[(.*)\]( \(Simulator\))?/.exec(line)) - .filter(notNull) - .filter(([_match, _name, _udid, isSim]) => !isSim) - .map(([_match, name, udid]) => { - return {udid: udid, type: 'physical', name: name}; - }), - ); + if (GK.get('flipper_use_idb_to_list_devices')) { + await memoize(checkIdbIsInstalled)(idbPath); + return safeExec(`${idbPath} list-targets --json`).then(({stdout}) => + stdout + .toString() + .split('\n') + .map((line) => line.trim()) + .map((line) => JSON.parse(line)) + .filter(({type}: IdbTarget) => type !== 'simulator') + .map((target: IdbTarget) => { + return {udid: target.udid, type: 'physical', name: target.name}; + }), + ); + } else { + await killOrphanedInstrumentsProcesses(); + return safeExec('instruments -s devices').then(({stdout}) => + stdout + .toString() + .split('\n') + .map((line) => line.trim()) + .map((line) => /(.+) \([^(]+\) \[(.*)\]( \(Simulator\))?/.exec(line)) + .filter(notNull) + .filter(([_match, _name, _udid, isSim]) => !isSim) + .map(([_match, name, udid]) => { + return {udid: udid, type: 'physical', name: name}; + }), + ); + } } async function push( @@ -70,7 +96,7 @@ async function push( dst: string, idbPath: string, ): Promise { - await checkIdbIsInstalled(idbPath); + await memoize(checkIdbIsInstalled)(idbPath); return wrapWithErrorMessage( reportPlatformFailures( safeExec( @@ -92,7 +118,7 @@ async function pull( dst: string, idbPath: string, ): Promise { - await checkIdbIsInstalled(idbPath); + await memoize(checkIdbIsInstalled)(idbPath); return wrapWithErrorMessage( reportPlatformFailures( safeExec( diff --git a/desktop/app/src/utils/listDevices.tsx b/desktop/app/src/utils/listDevices.tsx index 2b06dad47..5781475da 100644 --- a/desktop/app/src/utils/listDevices.tsx +++ b/desktop/app/src/utils/listDevices.tsx @@ -19,7 +19,7 @@ export async function listDevices(store: Store): Promise> { : []; const iOSDevices: BaseDevice[] = state.application .xcodeCommandLineToolsDetected - ? await getActiveDevicesAndSimulators() + ? await getActiveDevicesAndSimulators(store) : []; return [...androidDevices, ...iOSDevices]; } diff --git a/desktop/yarn.lock b/desktop/yarn.lock index 6c988a115..41a303c6b 100644 --- a/desktop/yarn.lock +++ b/desktop/yarn.lock @@ -2136,6 +2136,13 @@ dependencies: "@types/lodash" "*" +"@types/lodash.memoize@^4.1.6": + version "4.1.6" + resolved "https://registry.yarnpkg.com/@types/lodash.memoize/-/lodash.memoize-4.1.6.tgz#3221f981790a415cab1a239f25c17efd8b604c23" + integrity sha512-mYxjKiKzRadRJVClLKxS4wb3Iy9kzwJ1CkbyKiadVxejnswnRByyofmPMscFKscmYpl36BEEhCMPuWhA1R/1ZQ== + dependencies: + "@types/lodash" "*" + "@types/lodash@*", "@types/lodash@^4.14.150": version "4.14.157" resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.157.tgz#fdac1c52448861dfde1a2e1515dbc46e54926dc8"