Skip to content

[Profiler] Break metadata generation into multiple visitors#83033

Closed
robieta wants to merge 12 commits intogh/robieta/97/basefrom
gh/robieta/97/head
Closed

[Profiler] Break metadata generation into multiple visitors#83033
robieta wants to merge 12 commits intogh/robieta/97/basefrom
gh/robieta/97/head

Conversation

@robieta
Copy link
Contributor

@robieta robieta commented Aug 8, 2022 •
edited
Loading

Stack from ghstack (oldest at bottom):

This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: D38469409

[Profiler] Break metadata generation into multiple visitors
a9c2aee 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 8, 2022 •
edited
Loading

🔗 Helpful links

❌ 1 New Failures

As of commit adaa9cb (more details on the Dr. CI page):

Expand to see more
  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages

See GitHub Actions build pull / linux-docs / build-docs (python) (1/1)

Step: "Unknown" (full log | diagnosis details)

2022-08-20T07:56:57.8607326Z ##[error]The operation was canceled. 
2022-08-20T02:02:55.1351405Z copying images... [ 97%] _static/img/tensorboard/add_images.png
2022-08-20T02:02:55.1353234Z copying images... [100%] _static/img/tensorboard/add_hparam.png
2022-08-20T02:02:55.1354735Z 
2022-08-20T02:02:55.1580968Z copying static files... done
2022-08-20T02:02:55.1581564Z copying extra files... done
2022-08-20T02:02:55.5807206Z dumping search index in English (code: en)... done
2022-08-20T02:02:55.6842388Z dumping object inventory... done
2022-08-20T02:02:55.6845472Z build succeeded.
2022-08-20T02:02:55.6845641Z 
2022-08-20T02:02:55.6846642Z The HTML pages are in build/html.
2022-08-20T07:56:57.8607326Z ##[error]The operation was canceled.
2022-08-20T07:56:57.8636235Z Prepare all required actions
2022-08-20T07:56:57.8652478Z ##[group]Run ./.github/actions/chown-workspace
2022-08-20T07:56:57.8652798Z ##[endgroup]
2022-08-20T07:56:57.8665205Z ##[group]Run docker run --rm -v "$(pwd)":/v -w /v "${ALPINE_IMAGE}" chown -R "$(id -u):$(id -g)" .
2022-08-20T07:56:57.8665539Z �[36;1mdocker run --rm -v "$(pwd)":/v -w /v "${ALPINE_IMAGE}" chown -R "$(id -u):$(id -g)" .�[0m
2022-08-20T07:56:57.8675517Z shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
2022-08-20T07:56:57.8675735Z env:
2022-08-20T07:56:57.8675956Z   ALPINE_IMAGE: 308535385114.dkr.ecr.us-east-1.amazonaws.com/tool/alpine
2022-08-20T07:56:57.8676198Z ##[endgroup]
2022-08-20T07:56:59.3102036Z Post job cleanup.
This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

This was referenced Aug 8, 2022
Closed
Closed
Closed
robieta pushed a commit that referenced this pull request Aug 8, 2022
[Profiler] Break metadata generation into multiple visitors
4dccc06 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

ghstack-source-id: 163868318
Pull Request resolved: #83033
Update on "[Profiler] Break metadata generation into multiple visitors"
c65b8e9 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
robieta pushed a commit that referenced this pull request Aug 9, 2022
[Profiler] Break metadata generation into multiple visitors
c7a4764 
Pull Request resolved: #83033

This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)
ghstack-source-id: 163978390

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)
Update on "[Profiler] Break metadata generation into multiple visitors"
2efcecb 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
Update on "[Profiler] Break metadata generation into multiple visitors"
e1eda8a 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
Update on "[Profiler] Break metadata generation into multiple visitors"
c806888 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
robieta pushed a commit that referenced this pull request Aug 9, 2022
[Profiler] Break metadata generation into multiple visitors
1c292f9 
Pull Request resolved: #83033

This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)
ghstack-source-id: 164009732

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)
This was referenced Aug 17, 2022
Closed
Closed
Update on "[Profiler] Break metadata generation into multiple visitors"
5bdf3b1 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
robieta
robieta commented Aug 17, 2022
View reviewed changes
torch/csrc/autograd/profiler_kineto.cpp
while (parent && !parent_id.has_value()) {
parent->visit_if_base<PyExtraFieldsBase>(
[&](const auto& j) { parent_id = std::to_string(j.id_); });
parent = parent->parent_.lock();
Copy link
Contributor Author

@robieta robieta Aug 17, 2022

Choose a reason for hiding this comment

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

I discovered to my horror that this was missing meaning this PR could cause infinite loops. (And did for __torch_dispatch__.) Thus I added unit tests and confirmed that they looped until this line was added.

Taylor Robie added 4 commits August 17, 2022 13:46
Update on "[Profiler] Break metadata generation into multiple visitors"
18b5cdf 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
Update on "[Profiler] Break metadata generation into multiple visitors"
b26b52e 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
Update on "[Profiler] Break metadata generation into multiple visitors"
f5374a8 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
Update on "[Profiler] Break metadata generation into multiple visitors"
af03244 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
@robieta robieta added ciflow/trunk Trigger trunk jobs on your pull request ciflow/periodic Trigger jobs ran periodically on master (periodic.yml) on the PR labels Aug 19, 2022
Update on "[Profiler] Break metadata generation into multiple visitors"
cad09af 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
@robieta
Copy link
Contributor Author

robieta commented Aug 19, 2022

@pytorchbot merge -l

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Aug 19, 2022

@pytorchbot successfully started a merge job. Check the current status here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR have passed since you have added the ciflow/trunk label to your PR (ETA 0-4 Hours). If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Aug 20, 2022

Merge failed
Reason: Refusing to merge as mandatory check(s) pull failed for rule superuser
Raised by https://github.com/pytorch/pytorch/actions/runs/2892070462 If you believe this is an error, you can use the old behavior with @pytorchbot merge -g (optionally with the ciflow/trunk to get land checks) or use @pytorchbot merge -f "some reason here". For more information, see the bot wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

@robieta robieta removed ciflow/trunk Trigger trunk jobs on your pull request ciflow/periodic Trigger jobs ran periodically on master (periodic.yml) on the PR labels Aug 20, 2022
Update on "[Profiler] Break metadata generation into multiple visitors"
adaa9cb 
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Differential Revision: [D38469409](https://our.internmc.facebook.com/intern/diff/D38469409/)

[ghstack-poisoned]
@robieta robieta added the release notes: profiler release notes category label Aug 21, 2022
facebook-github-bot pushed a commit that referenced this pull request Aug 23, 2022
@facebook-github-bot
[Profiler] Break metadata generation into multiple visitors (#83033) (#…
0c7fff4 
…83033)

Summary:
This is a no-op change which establishes a base class to handle Result to Kineto details, and then splits the existing logging logic. (With the idea that at some point we'll probably conditionally run things to manage trace size.)

Pull Request resolved: #83033
Approved by: https://github.com/aaronenyeshi

Test Plan:
contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/37f91d700bcb80c5f254c734a420ad89928771f4

Original Phabricator Test Plan:
Existing unit tests.

Reviewed By: aaronenyeshi, seemethere

Differential Revision: D38469409

Pulled By: robieta

fbshipit-source-id: 143a45044782e7120802948fba3274e905e65af8
@facebook-github-bot facebook-github-bot deleted the gh/robieta/97/head branch August 24, 2022 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aaronenyeshi aaronenyeshi aaronenyeshi approved these changes

Assignees

No one assigned

Labels

cla signed release notes: profiler release notes category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@robieta @facebook-github-bot @pytorchmergebot @aaronenyeshi