Skip to content

Commit

Permalink
fix: Improve Big Sur browser detection performance (#17807)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisbreiding authored Aug 19, 2021
1 parent e9656df commit aa3ea48
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 10 deletions.
4 changes: 2 additions & 2 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ macBuildFilters: &macBuildFilters
branches:
only:
- develop
- tgriesser/fix/mac-build
- issue-17773-slow-browser-detection

defaults: &defaults
parallelism: 1
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions packages/launcher/lib/darwin/detection-workaround.js
Original file line number Diff line number Diff line change
@@ -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))
})
23 changes: 22 additions & 1 deletion packages/launcher/lib/darwin/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> {
Expand Down Expand Up @@ -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<FoundBrowser[]> {
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)
}
34 changes: 33 additions & 1 deletion packages/launcher/lib/detect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -137,13 +138,44 @@ function checkOneBrowser (browser: Browser): Promise<boolean | FoundBrowser> {
}

/** returns list of detected browsers */
export const detect = (goalBrowsers?: Browser[]): Bluebird<FoundBrowser[]> => {
export const detect = (goalBrowsers?: Browser[], useDarwinWorkaround = true): Bluebird<FoundBrowser[]> => {
// 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)
})
Expand Down
2 changes: 1 addition & 1 deletion packages/launcher/lib/windows/index.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
5 changes: 3 additions & 2 deletions packages/launcher/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -36,4 +37,4 @@
"lib"
],
"types": "index.ts"
}
}
24 changes: 23 additions & 1 deletion packages/launcher/test/unit/detect_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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)
})
})
})
4 changes: 3 additions & 1 deletion packages/launcher/test/unit/linux_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' })

Expand Down Expand Up @@ -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]) => {
Expand Down
1 change: 1 addition & 0 deletions packages/server/lib/util/find_system_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,6 @@ function findNodePathAndVersion () {
}

module.exports = {
findNodeInFullPath,
findNodePathAndVersion,
}
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -34688,7 +34688,7 @@ [email protected]:
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==
Expand Down

4 comments on commit aa3ea48

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on aa3ea48 Aug 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/8.3.1/circle-develop-aa3ea4871725b563a75111cfd0f347030f7c4930/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on aa3ea48 Aug 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AppVeyor has built the win32 ia32 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/8.3.1/appveyor-develop-aa3ea4871725b563a75111cfd0f347030f7c4930/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on aa3ea48 Aug 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AppVeyor has built the win32 x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/8.3.1/appveyor-develop-aa3ea4871725b563a75111cfd0f347030f7c4930/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on aa3ea48 Aug 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/8.3.1/circle-develop-aa3ea4871725b563a75111cfd0f347030f7c4930/cypress.tgz

Please sign in to comment.