Skip to content

Add config for incoming XPU scope profiler #1180

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
Open
moksiuc wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add config for incoming XPU scope profiler #1180

moksiuc wants to merge 6 commits into from

Conversation

@moksiuc
Copy link
Contributor

@moksiuc moksiuc commented Nov 17, 2025 •
edited
Loading

Summary:

Added new class XpuptiScopeProfilerConfig derived from AbstractConfig for configuration of the new scope profiler.
Added gtest

@meta-cla meta-cla bot added the cla signed label Nov 17, 2025
@moksiuc moksiuc changed the title Config for scope profiler Add config for incoming XPU scope profiler Nov 17, 2025
@moksiuc
Copy link
Contributor Author

moksiuc commented Nov 17, 2025

@EikanWang, @gujinghui
Second PR extracted from:
#1174

gujinghui
gujinghui reviewed Nov 18, 2025
View reviewed changes
@moksiuc
Code review fix
71b3f02 
- place of config initialization
@moksiuc moksiuc requested a review from gujinghui November 18, 2025 13:31
@moksiuc moksiuc marked this pull request as ready for review November 18, 2025 13:31
@gujinghui gujinghui mentioned this pull request Nov 19, 2025
Open
@moksiuc
Copy link
Contributor Author

moksiuc commented Dec 1, 2025

@sraikund16, could you review or suggest who else could do it ?

@meta-codesync
Copy link

meta-codesync bot commented Dec 1, 2025

@malfet has imported this pull request. If you are a Meta employee, you can view this in D88098676.

@moksiuc
Copy link
Contributor Author

moksiuc commented Dec 3, 2025

@malfet what are further steps until this PR has chance to be merged ?

aaronenyeshi
aaronenyeshi approved these changes Dec 9, 2025
View reviewed changes
Copy link
Member

@aaronenyeshi aaronenyeshi left a comment

Choose a reason for hiding this comment

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

Review automatically exported from Phabricator review in Meta.

@gujinghui
Copy link

gujinghui commented Dec 16, 2025

@malfet @aaronenyeshi @sraikund16
Could you help merge this PR? Thanks.

@sraikund16
Copy link
Contributor

sraikund16 commented Dec 16, 2025

@gujinghui sure I'll take it over

@sraikund16
Copy link
Contributor

sraikund16 commented Dec 16, 2025

@moksiuc can you merge this change moksiuc#5

Also I added a linter here: #1206

Hopefully we will not need to deal with this going forward

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Dec 17, 2025

@moksiuc has updated the pull request. You must reimport the pull request before landing.

@sraikund16
Copy link
Contributor

sraikund16 commented Dec 17, 2025

@moksiuc it looks like there are some conflicts can you merge with main and fix them?

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Dec 18, 2025

@moksiuc has updated the pull request. You must reimport the pull request before landing.

@moksiuc
Copy link
Contributor Author

moksiuc commented Dec 18, 2025 •
edited
Loading

@moksiuc it looks like there are some conflicts can you merge with main and fix them?

@sraikund16 conflicts resolved

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

Reviewers

@malfet malfet malfet approved these changes

@aaronenyeshi aaronenyeshi aaronenyeshi approved these changes

+1 more reviewer

@gujinghui gujinghui gujinghui approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@moksiuc @gujinghui @sraikund16 @facebook-github-bot @malfet @aaronenyeshi