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.

Sign up for GitHub

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

Jump to bottom

build: enable Perfetto in Chromium #41880

Merged
merged 5 commits into from
Apr 19, 2024
Merged

build: enable Perfetto in Chromium #41880

merged 5 commits into from
Apr 19, 2024

Conversation

codebytere
Copy link
Member

Description of Change

Enable perfetto with Node.js and turn off use_perfetto_client_library = False. Node.js disables perfetto by default but is broken on build - they don't currently add guards for V8_USE_PERFETTO and upstream only defines certain functions (ex. GetCategoryGroupEnabled)on v8::TracingController if perfetto is disabled. Electron already had minimal to no support for Node.js trace events, so the impact of adding associated guards there should be relatively small.

Checklist

Release Notes

Notes: none

@codebytere codebytere requested review from nornagon and jkleinsc April 17, 2024 13:59
@codebytere codebytere requested a review from a team as a code owner April 17, 2024 13:59
@codebytere
Copy link
Member Author

@nornagon i'm observing with Perfetto enabled that this test passes even when I delete the entirety of TracingControllerImpl - is there something i'm maybe missing? Otherwise might try deleting that in here too.

@nornagon
Copy link
Member

nornagon commented Apr 17, 2024 •
edited
Loading

@nornagon i'm observing with Perfetto enabled that this test passes even when I delete the entirety of TracingControllerImpl - is there something i'm maybe missing? Otherwise might try deleting that in here too.

That test looks pretty solid to me, so I'd trust the results. It looks like most of TracingControllerImpl is not compiled when perfetto is enabled, so it makes sense that deleting it wouldn't have any effect when perfetto is enabled.

[EDIT]: oh, whoops, I see you linked to the version of the file in this PR which adds the #ifdef. But still, the test is fairly strong so I think we're okay to trust it.

@codebytere codebytere force-pushed the perfetto-enable-minimal branch 3 times, most recently from 01d6fec to efa3ee7 Compare April 17, 2024 22:45
@codebytere codebytere force-pushed the perfetto-enable-minimal branch 2 times, most recently from 53fd714 to ff3258f Compare April 19, 2024 00:23
@codebytere codebytere force-pushed the perfetto-enable-minimal branch from ff3258f to b5bb543 Compare April 19, 2024 01:24
@codebytere codebytere added target/31-x-y PR should also be added to the "31-x-y" branch. semver/none and removed no-backport semver/none target/31-x-y PR should also be added to the "31-x-y" branch. labels Apr 19, 2024
@jkleinsc jkleinsc merged commit 39bf441 into main Apr 19, 2024
21 checks passed
@jkleinsc jkleinsc deleted the perfetto-enable-minimal branch April 19, 2024 15:07
@release-clerk Release Clerk
Copy link

release-clerk bot commented Apr 19, 2024

No Release Notes

@trop trop bot mentioned this pull request Apr 19, 2024
Merged
@trop
Copy link
Contributor

trop bot commented Apr 19, 2024

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

@trop trop bot added in-flight/31-x-y merged/31-x-y PR was merged to the "31-x-y" branch. and removed target/31-x-y PR should also be added to the "31-x-y" branch. in-flight/31-x-y labels Apr 19, 2024
ckerr added a commit that referenced this pull request Aug 19, 2024
@ckerr
chore: remove unused ConvertableToTraceFormatWrapper
c729d43 
Last use removed in Apr 2024 (39bf441, #41880)
@ckerr ckerr mentioned this pull request Aug 19, 2024
Merged
3 tasks
jkleinsc pushed a commit that referenced this pull request Aug 20, 2024
@ckerr
chore: remove unused ConvertableToTraceFormatWrapper (#43356)
a2de94d 
* chore: remove unused ConvertableToTraceFormatWrapper

Last use removed in Apr 2024 (39bf441, #41880)

* fixup! chore: remove unused ConvertableToTraceFormatWrapper

remove now-unused trace_event.h header, too
ckerr added a commit that referenced this pull request Aug 20, 2024
@ckerr
chore: remove unused ConvertableToTraceFormatWrapper (#43356)
069c923 
* chore: remove unused ConvertableToTraceFormatWrapper

Last use removed in Apr 2024 (39bf441, #41880)

* fixup! chore: remove unused ConvertableToTraceFormatWrapper

remove now-unused trace_event.h header, too
ckerr added a commit that referenced this pull request Aug 20, 2024
@ckerr
chore: remove unused ConvertableToTraceFormatWrapper (33-x-y) (#43378)
2d24045 
chore: remove unused ConvertableToTraceFormatWrapper (#43356)

* chore: remove unused ConvertableToTraceFormatWrapper

Last use removed in Apr 2024 (39bf441, #41880)

* fixup! chore: remove unused ConvertableToTraceFormatWrapper

remove now-unused trace_event.h header, too
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkleinsc jkleinsc jkleinsc approved these changes

@nornagon nornagon nornagon approved these changes

Assignees
No one assigned
Labels
merged/31-x-y PR was merged to the "31-x-y" branch. semver/none
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@codebytere @nornagon @jkleinsc