-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Improve Big Sur browser detection performance #17807
Conversation
Thanks for taking the time to open a PR!
|
👏 👏 👏
…Sent from my iPhone
On Aug 19, 2021, at 10:17, cypress-bot[bot] ***@***.***> wrote:
Thanks for taking the time to open a PR!
Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
Nice. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do what we can because we must. LGTM
BTW, was not able to test this on an actual Big Sur install as part of my review, my VM is stuck on Catalina. |
An upstream fix is coming: electron/electron#26143 (comment) |
User facing changelog
Additional details
The root of the issue is this bug in Electron, which causes spawning child processes to take 300-1000ms instead of the usual <30ms when using MacOS Big Sur and Electron 12+. We spawn a child process for each binary of each browser, plus potential fallback lookups if any of the standard ones fail. This causes the browser detection on Big Sur to take upwards of 10 seconds.
We considered using the Electron patch in that issue, but it's not a perfect solution (it was reverted in libuv due to failing tests), and would require us to build Electron ourselves, which is a large amount of work and responsibility.
As a workaround, we spawn a single child process (only incurring the performance penalty once instead of N+ times), and use it to call into our browser detection code from the user's Node.js. Since it's outside Electron, it doesn't suffer from the same bug and performs quickly. This reduces the time taken for browser detection from ~10s to ~1s.
How has the user experience changed?
Users on Big Sur will see projects open much quicker.
PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?