From aa3ea4871725b563a75111cfd0f347030f7c4930 Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Thu, 19 Aug 2021 14:15:55 -0400 Subject: [PATCH] fix: Improve Big Sur browser detection performance (#17807) --- circle.yml | 4 +-- .../lib/darwin/detection-workaround.js | 6 ++++ packages/launcher/lib/darwin/util.ts | 23 ++++++++++++- packages/launcher/lib/detect.ts | 34 ++++++++++++++++++- packages/launcher/lib/windows/index.ts | 2 +- packages/launcher/package.json | 5 +-- packages/launcher/test/unit/detect_spec.ts | 24 ++++++++++++- packages/launcher/test/unit/linux_spec.ts | 4 ++- packages/server/lib/util/find_system_node.js | 1 + yarn.lock | 2 +- 10 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 packages/launcher/lib/darwin/detection-workaround.js diff --git a/circle.yml b/circle.yml index 2117d90f6b3e..5866b1d4c5eb 100644 --- a/circle.yml +++ b/circle.yml @@ -8,7 +8,7 @@ macBuildFilters: &macBuildFilters branches: only: - develop - - tgriesser/fix/mac-build + - issue-17773-slow-browser-detection defaults: &defaults parallelism: 1 @@ -1489,7 +1489,7 @@ jobs: - run: name: Check current branch to persist artifacts command: | - if [[ "$CIRCLE_BRANCH" != "develop" ]]; then + if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "issue-17773-slow-browser-detection" ]]; then echo "Not uploading artifacts or posting install comment for this branch." circleci-agent step halt fi diff --git a/packages/launcher/lib/darwin/detection-workaround.js b/packages/launcher/lib/darwin/detection-workaround.js new file mode 100644 index 000000000000..89f78738eb63 --- /dev/null +++ b/packages/launcher/lib/darwin/detection-workaround.js @@ -0,0 +1,6 @@ +const { detect } = require('../detect') + +detect(undefined, false).then((browsers) => { + // eslint-disable-next-line no-console + console.log(JSON.stringify(browsers, null, 2)) +}) diff --git a/packages/launcher/lib/darwin/util.ts b/packages/launcher/lib/darwin/util.ts index e7ab530b2ffa..4992fb857d82 100644 --- a/packages/launcher/lib/darwin/util.ts +++ b/packages/launcher/lib/darwin/util.ts @@ -2,9 +2,13 @@ import { log } from '../log' import { notInstalledErr } from '../errors' import { prop, tap } from 'ramda' import { utils } from '../utils' -import fs from 'fs-extra' +import * as fs from 'fs-extra' +import * as os from 'os' import * as path from 'path' import * as plist from 'plist' +import * as semver from 'semver' +import { FoundBrowser } from '../types' +import * as findSystemNode from '@packages/server/lib/util/find_system_node' /** parses Info.plist file from given application and returns a property */ export function parsePlist (p: string, property: string): Promise { @@ -96,3 +100,20 @@ export function findApp ({ appName, executable, appId, versionProperty }: FindAp return tryMdFind().catch(tryFullApplicationFind) } + +export function needsDarwinWorkaround (): boolean { + return os.platform() === 'darwin' && semver.gte(os.release(), '20.0.0') +} + +export async function darwinDetectionWorkaround (): Promise { + const nodePath = await findSystemNode.findNodeInFullPath() + let args = ['./detection-workaround.js'] + + if (process.env.CYPRESS_INTERNAL_ENV === 'development') { + args = ['-r', '@packages/ts/register.js'].concat(args) + } + + const { stdout } = await utils.execa(nodePath, args, { cwd: __dirname }) + + return JSON.parse(stdout) +} diff --git a/packages/launcher/lib/detect.ts b/packages/launcher/lib/detect.ts index 83114d22d0f4..d489f333e2b5 100644 --- a/packages/launcher/lib/detect.ts +++ b/packages/launcher/lib/detect.ts @@ -4,6 +4,7 @@ import os from 'os' import { flatten, merge, pick, props, tap, uniqBy } from 'ramda' import { browsers } from './browsers' import * as darwinHelper from './darwin' +import { needsDarwinWorkaround, darwinDetectionWorkaround } from './darwin/util' import { notDetectedAtPathErr } from './errors' import * as linuxHelper from './linux' import { log } from './log' @@ -137,13 +138,44 @@ function checkOneBrowser (browser: Browser): Promise { } /** returns list of detected browsers */ -export const detect = (goalBrowsers?: Browser[]): Bluebird => { +export const detect = (goalBrowsers?: Browser[], useDarwinWorkaround = true): Bluebird => { // we can detect same browser under different aliases // tell them apart by the name and the version property if (!goalBrowsers) { goalBrowsers = browsers } + // BigSur (darwin 20.x) and Electron 12+ cause huge performance issues when + // spawning child processes, which is the way we find browsers via execa. + // The performance cost is multiplied by the number of binary variants of + // each browser plus any fallback lookups we do. + // The workaround gets around this by breaking out of the bundled Electron + // Node.js and using the user's Node.js if possible. It only pays the cost + // of spawning a single child process instead of multiple. If this fails, + // we fall back to to the slower, default method + // https://github.com/cypress-io/cypress/issues/17773 + if (useDarwinWorkaround && needsDarwinWorkaround()) { + log('using darwin detection workaround') + if (log.enabled) { + // eslint-disable-next-line no-console + console.time('time taken detecting browsers (darwin workaround)') + } + + return Bluebird.resolve(darwinDetectionWorkaround()) + .catch((err) => { + log('darwin workaround failed, falling back to normal detection') + log(err.stack) + + return detect(goalBrowsers, false) + }) + .finally(() => { + if (log.enabled) { + // eslint-disable-next-line no-console + console.timeEnd('time taken detecting browsers (darwin workaround)') + } + }) + } + const removeDuplicates = uniqBy((browser: FoundBrowser) => { return props(['name', 'version'], browser) }) diff --git a/packages/launcher/lib/windows/index.ts b/packages/launcher/lib/windows/index.ts index 16dbb40af493..3fa8a033e38d 100644 --- a/packages/launcher/lib/windows/index.ts +++ b/packages/launcher/lib/windows/index.ts @@ -1,4 +1,4 @@ -import fse from 'fs-extra' +import * as fse from 'fs-extra' import os from 'os' import { join, normalize, win32 } from 'path' import { tap, trim, prop } from 'ramda' diff --git a/packages/launcher/package.json b/packages/launcher/package.json index ea74e906adda..a148916a7f52 100644 --- a/packages/launcher/package.json +++ b/packages/launcher/package.json @@ -19,7 +19,8 @@ "fs-extra": "8.1.0", "lodash": "4.17.21", "plist": "3.0.1", - "ramda": "0.27.1" + "ramda": "0.27.1", + "semver": "7.3.5" }, "devDependencies": { "@packages/ts": "0.0.0-development", @@ -36,4 +37,4 @@ "lib" ], "types": "index.ts" -} \ No newline at end of file +} diff --git a/packages/launcher/test/unit/detect_spec.ts b/packages/launcher/test/unit/detect_spec.ts index 139d8c7728b4..78057aa9d963 100644 --- a/packages/launcher/test/unit/detect_spec.ts +++ b/packages/launcher/test/unit/detect_spec.ts @@ -4,9 +4,10 @@ import { goalBrowsers } from '../fixtures' import { expect } from 'chai' import { utils } from '../../lib/utils' import sinon, { SinonStub } from 'sinon' -const os = require('os') +import os from 'os' import { log } from '../log' import { project } from 'ramda' +import * as darwinUtil from '../../lib/darwin/util' const isWindows = () => { return os.platform() === 'win32' @@ -168,4 +169,25 @@ describe('browser detection', () => { .and.contain('Firefox newer than or equal to 86') }) }) + + // https://github.com/cypress-io/cypress/issues/17773 + context('darwin performance workaround', () => { + let browsers + + beforeEach(() => { + sinon.stub(os, 'platform').returns('darwin') + sinon.stub(os, 'release').returns('20.0.0') + + browsers = [] + + sinon.stub(darwinUtil, 'darwinDetectionWorkaround').resolves(browsers) + }) + + it('uses workaround when on darwin 20.0.0+', async () => { + const result = await detect() + + expect(darwinUtil.darwinDetectionWorkaround).to.be.called + expect(result).to.equal(browsers) + }) + }) }) diff --git a/packages/launcher/test/unit/linux_spec.ts b/packages/launcher/test/unit/linux_spec.ts index a56a968d5acc..cc8db36aa36f 100644 --- a/packages/launcher/test/unit/linux_spec.ts +++ b/packages/launcher/test/unit/linux_spec.ts @@ -18,6 +18,9 @@ describe('linux browser detection', () => { sinon.restore() execa = sinon.stub(utils, 'getOutput') + sinon.stub(os, 'platform').returns('linux') + sinon.stub(os, 'release').returns('1.0.0') + execa.withArgs('test-browser', ['--version']) .resolves({ stdout: 'test-browser v100.1.2.3' }) @@ -50,7 +53,6 @@ describe('linux browser detection', () => { execa.withArgs('chromium', ['--version']) .resolves({ stdout: 'Chromium 64.2.3 snap' }) - sinon.stub(os, 'platform').returns('linux') sinon.stub(os, 'homedir').returns('/home/foo') const checkBrowser = ([browser]) => { diff --git a/packages/server/lib/util/find_system_node.js b/packages/server/lib/util/find_system_node.js index cec8d87d338d..21d02671ba41 100644 --- a/packages/server/lib/util/find_system_node.js +++ b/packages/server/lib/util/find_system_node.js @@ -79,5 +79,6 @@ function findNodePathAndVersion () { } module.exports = { + findNodeInFullPath, findNodePathAndVersion, } diff --git a/yarn.lock b/yarn.lock index aec8691a875c..54a192136dd2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -34688,7 +34688,7 @@ semver@7.3.4: dependencies: lru-cache "^6.0.0" -semver@^7.0.0, semver@^7.1.1, semver@^7.1.2, semver@^7.2.1, semver@^7.3.2, semver@^7.3.4, semver@^7.3.5: +semver@7.3.5, semver@^7.0.0, semver@^7.1.1, semver@^7.1.2, semver@^7.2.1, semver@^7.3.2, semver@^7.3.4, semver@^7.3.5: version "7.3.5" resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.5.tgz#0b621c879348d8998e4b0e4be94b3f12e6018ef7" integrity sha512-PoeGJYh8HK4BTO/a9Tf6ZG3veo/A7ZVsYrSA6J8ny9nb3B1VrpkuN+z9OE5wfE5p6H4LchYZsegiQgbJD94ZFQ==