Skip to content
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

chore: add a TRACE call for crash_reporter::Start() #34268

Merged

Conversation

RaisinTen
Copy link
Member

Description of Change

Initializing the crashReporter takes around 620 milliseconds on Intel
macOS. I have sent a CL to crashpad to partially fix the performance
issue in https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3641386.
It would be beneficial to log the performance impact of this function in
the traces in case this slows down in the future.

Signed-off-by: Darshan Sen raisinten@gmail.com

Checklist

Release Notes

Notes: Added a TRACE call named crash_reporter::Start under the electron category for crash_reporter::Start().

Initializing the crashReporter takes around 620 milliseconds on Intel
macOS. I have sent a CL to crashpad to partially fix the performance
issue in
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3641386.
It would be beneficial to log the performance impact of this function in
the traces in case this slows down in the future.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label May 18, 2022
@RaisinTen
Copy link
Member Author

I think this is semver-minor, so shouldn't this be backported to all the supported release lines?

@RaisinTen
Copy link
Member Author

ping @nornagon since you reviewed #23501 which also adds trace event calls to another api

@codebytere codebytere requested a review from nornagon May 23, 2022 12:51
@codebytere
Copy link
Member

codebytere commented May 23, 2022

@RaisinTen this doesn't affect the API surfaced to end users, so we'd classify this as semver-patch. I can add labels to send it to current release lines though!

@codebytere codebytere added the semver/patch backwards-compatible bug fixes label May 23, 2022
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label May 23, 2022
@VerteDinde VerteDinde merged commit df9383c into electron:main May 23, 2022
@release-clerk
Copy link

release-clerk bot commented May 23, 2022

Release Notes Persisted

Added a TRACE call named crash_reporter::Start under the electron category for crash_reporter::Start().

@trop
Copy link
Contributor

trop bot commented May 23, 2022

I have automatically backported this PR to "17-x-y", please check out #34324

@trop
Copy link
Contributor

trop bot commented May 23, 2022

I have automatically backported this PR to "18-x-y", please check out #34325

@trop
Copy link
Contributor

trop bot commented May 23, 2022

I have automatically backported this PR to "19-x-y", please check out #34326

@RaisinTen RaisinTen deleted the chore/add-trace-for-crash_reporter-Start branch May 23, 2022 16:06
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
chore: add a TRACE call for crash_reporter::Start()

Initializing the crashReporter takes around 620 milliseconds on Intel
macOS. I have sent a CL to crashpad to partially fix the performance
issue in
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3641386.
It would be beneficial to log the performance impact of this function in
the traces in case this slows down in the future.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants