Skip to content

fix: performance problem in crashReporter.start() on macOS - #34637

Merged
zcbenz merged 5 commits into
17-x-yfrom
trop/17-x-y-bp-fix-performance-problem-in-crashreporter-start-on-macos-1655699550456
Jun 20, 2022
Merged

fix: performance problem in crashReporter.start() on macOS#34637
zcbenz merged 5 commits into
17-x-yfrom
trop/17-x-y-bp-fix-performance-problem-in-crashreporter-start-on-macos-1655699550456

Conversation

@trop

@trop trop Bot commented Jun 20, 2022

Copy link
Copy Markdown
Contributor

Backport of #34609

See that PR for details.

Notes: Fixes a performance problem in crashReporter.start() on macOS.

This change reduces the duration of crashReporter.start() on Intel macOS
from 622 milliseconds to 257 milliseconds!

Backports https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3641386

  posix: Replace DoubleForkAndExec() with ForkAndSpawn()

  The DoubleForkAndExec() function was taking over 622 milliseconds to run
  on macOS 11 (BigSur) on Intel i5-1038NG7. I did some debugging by adding
  some custom traces and found that the fork() syscall is the bottleneck
  here, i.e., the first fork() takes around 359 milliseconds and the
  nested fork() takes around 263 milliseconds. Replacing the nested fork()
  and exec() with posix_spawn() reduces the time consumption to 257
  milliseconds!

  See libuv/libuv#3064 to know why fork() is so
  slow on macOS and why posix_spawn() is a better replacement.

  Another point to note is that even base::LaunchProcess() from Chromium
  calls posix_spawnp() on macOS -
  https://source.chromium.org/chromium/chromium/src/+/8f8d82dea0fa8f11f57c74dbb65126f8daba58f7:base/process/launch_mac.cc;l=295-296

  Change-Id: I25c6ee9629a1ae5d0c32b361b56a1ce0b4b0fd26
  Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3641386
  Reviewed-by: Mark Mentovai <mark@chromium.org>
  Commit-Queue: Mark Mentovai <mark@chromium.org>

Fixes: #34321
Signed-off-by: Darshan Sen <raisinten@gmail.com>
@trop
trop Bot requested review from a team as code owners June 20, 2022 04:32
@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label Jun 20, 2022
@trop trop Bot added 17-x-y backport This is a backport PR semver/patch backwards-compatible bug fixes labels Jun 20, 2022
@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label Jun 20, 2022

@codebytere codebytere left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Build failure - bad macros. They'll need updating.

@codebytere codebytere left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

........nevermind, got fixed the literal moment i reviewed it.

@zcbenz
zcbenz merged commit a14c789 into 17-x-y Jun 20, 2022
@zcbenz
zcbenz deleted the trop/17-x-y-bp-fix-performance-problem-in-crashreporter-start-on-macos-1655699550456 branch June 20, 2022 10:39
@release-clerk

release-clerk Bot commented Jun 20, 2022

Copy link
Copy Markdown

Release Notes Persisted

Fixes a performance problem in crashReporter.start() on macOS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

17-x-y backport This is a backport PR semver/patch backwards-compatible bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants