Skip to content

[Profiler] Make KinetoEvent a view of Result (Part 2, python and stacks)#81320

Closed
robieta wants to merge 16 commits intogh/robieta/81/basefrom
gh/robieta/81/head
Closed

[Profiler] Make KinetoEvent a view of Result (Part 2, python and stacks)#81320
robieta wants to merge 16 commits intogh/robieta/81/basefrom
gh/robieta/81/head

Conversation

@robieta
Copy link
Contributor

@robieta robieta commented Jul 12, 2022 •
edited
Loading

Stack from ghstack (oldest at bottom):

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in EventFieldsVisitor and just lean on KinetoEvent.stack().

Differential Revision: D37481561

[Profiler] Make KinetoEvent a view of Result (Part 2, python and stacks)
9639972 
The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
@robieta robieta requested review from albanD and soulitzer as code owners July 12, 2022 14:59
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jul 12, 2022 •
edited
Loading

🔗 Helpful links

❌ 1 New Failures

As of commit 76c5d5a (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 periodic / ios-12-5-1-arm64-custom-ops / build (1/1)

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

2022-08-08T05:09:56.6079800Z ##[error]The operation was canceled. 
2022-08-08T02:18:27.1190370Z 
2022-08-08T02:18:27.1218720Z CodeSign /Users/runner/work/pytorch/pytorch/ios/TestApp/build/Release-iphoneos/TestApp.app (in target 'TestApp' from project 'TestApp')
2022-08-08T02:18:27.1247390Z     cd /Users/runner/work/pytorch/pytorch/ios/TestApp
2022-08-08T02:18:27.1248370Z     export CODESIGN_ALLOCATE\=/Applications/Xcode_13.4.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/codesign_allocate
2022-08-08T02:18:27.1249180Z     
2022-08-08T02:18:27.1250280Z     Signing Identity:     "Apple Development: Tao Xu (Q7PV3L5FKY)"
2022-08-08T02:18:27.1250980Z     Provisioning Profile: "PyTorch_CI_2022"
2022-08-08T02:18:27.1252160Z                           (4aacbe80-cff0-4415-9ae8-c84476678cd5)
2022-08-08T02:18:27.1252720Z     
2022-08-08T02:18:27.1253770Z     /usr/bin/codesign --force --sign 31F7F74EFAF97190E9C2F6C230C27D3AC1861A63 --entitlements /Users/runner/work/pytorch/pytorch/ios/TestApp/build/TestApp.build/Release-iphoneos/TestApp.build/TestApp.app.xcent --timestamp\=none --generate-entitlement-der /Users/runner/work/pytorch/pytorch/ios/TestApp/build/Release-iphoneos/TestApp.app
2022-08-08T05:09:56.6079800Z ##[error]The operation was canceled.
2022-08-08T05:09:56.6207940Z Post job cleanup.
2022-08-08T05:09:56.6299560Z Post job cleanup.
2022-08-08T05:09:57.2372450Z [command]/usr/local/bin/git version
2022-08-08T05:09:57.7936420Z git version 2.37.1
2022-08-08T05:09:57.7977030Z Copying '/Users/runner/.gitconfig' to '/Users/runner/work/_temp/4e2eeba4-0b7a-4c6c-b9a3-77b5460f717c/.gitconfig'
2022-08-08T05:09:57.7978380Z Temporarily overriding HOME='/Users/runner/work/_temp/4e2eeba4-0b7a-4c6c-b9a3-77b5460f717c' before making global git config changes
2022-08-08T05:09:57.7979870Z Adding repository directory to the temporary git global config as a safe directory
2022-08-08T05:09:57.7981720Z [command]/usr/local/bin/git config --global --add safe.directory /Users/runner/work/pytorch/pytorch
2022-08-08T05:09:57.7983490Z [command]/usr/local/bin/git config --local --name-only --get-regexp core\.sshCommand
2022-08-08T05:09:57.7984930Z [command]/usr/local/bin/git submodule foreach --recursive git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :
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 Jul 12, 2022
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@albanD albanD removed their request for review July 21, 2022 10:41
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
574be85 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
This was referenced Jul 22, 2022
Closed
Closed
Closed
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
240a3a3 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
robieta
robieta commented Jul 29, 2022
View reviewed changes
torch/csrc/autograd/profiler_kineto.cpp Outdated
}

const c10::ArrayRef<std::string> KinetoEvent::stack() const {
auto get = [&](const auto& i) {
Copy link
Contributor Author

@robieta robieta Jul 29, 2022

Choose a reason for hiding this comment

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

C++: Just use lambdas, they simplify everything.
Also C++: "This line is missing -> auto& so we made a copy and now ASAN is mad."

Taylor Robie added 2 commits July 29, 2022 17:23
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
c50a5e0 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
d59dae7 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
robieta pushed a commit that referenced this pull request Jul 30, 2022
[Profiler] Make KinetoEvent a view of Result (Part 2, python and stacks)
50d9b6c 
Pull Request resolved: #81320

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.
ghstack-source-id: 162969387

Differential Revision: [D37481561](https://our.internmc.facebook.com/intern/diff/D37481561/)
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
9d79ee4 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
25e9a93 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
5516e19 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
f791164 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
robieta pushed a commit that referenced this pull request Aug 1, 2022
[Profiler] Make KinetoEvent a view of Result (Part 2, python and stacks)
8211fdc 
Pull Request resolved: #81320

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.
ghstack-source-id: 163104855

Differential Revision: [D37481561](https://our.internmc.facebook.com/intern/diff/D37481561/)
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
6450589 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
@robieta robieta mentioned this pull request Aug 1, 2022
Closed
Taylor Robie added 2 commits August 3, 2022 10:40
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
ce02537 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
17afca0 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
pytorchmergebot pushed a commit that referenced this pull request Aug 5, 2022
@pytorchmergebot
[Profiler] Make KinetoEvent a view of Result (Part 2, python and stac…
32cc923 
…ks) (#81320)

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

Differential Revision: [D37481561](https://our.internmc.facebook.com/intern/diff/D37481561/)
Pull Request resolved: #81320
Approved by: https://github.com/pbelevich
Taylor Robie added 2 commits August 5, 2022 08:08
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
ff9b238 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
157f4f2 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[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 with-ssh and removed with-ssh labels Aug 6, 2022
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
b95a8a6 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
robieta pushed a commit that referenced this pull request Aug 6, 2022
[Profiler] Make KinetoEvent a view of Result (Part 2, python and stacks)
1290ea5 
Pull Request resolved: #81320

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.
ghstack-source-id: 163748936

Differential Revision: [D37481561](https://our.internmc.facebook.com/intern/diff/D37481561/)
Update on "[Profiler] Make KinetoEvent a view of Result (Part 2, pyth…
76c5d5a 
…on and stacks)"

The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

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

[ghstack-poisoned]
@robieta robieta added the release notes: profiler release notes category label Aug 8, 2022
This was referenced Aug 8, 2022
Closed
Closed
facebook-github-bot pushed a commit that referenced this pull request Aug 9, 2022
@facebook-github-bot
[Profiler] Make KinetoEvent a view of Result (Part 2, python and stac…
4e51c3b 
…ks) (#81320) (#81320)

Summary:
The semantics of python in KinetoEvent are sort of crazy because values can come either from metadata captured by JIT or from the python tracer. Things aren't helped by the fact that we encode the python hierarchy in the chrome trace for tensorboard, so we have to do a tree traversal with skips based on types.

By simply constructing the Python stack in KinetoEvent's ctor we're able to skip a lot of the complexity in `EventFieldsVisitor` and just lean on `KinetoEvent.stack()`.

Pull Request resolved: #81320
Approved by: https://github.com/pbelevich

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

Original Phabricator Test Plan:
Existing unit tests

Reviewed By: chaekit, seemethere, kit1980

Differential Revision: D37481561

Pulled By: robieta

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

Reviewers

@soulitzer soulitzer Awaiting requested review from soulitzer

1 more reviewer

@pbelevich pbelevich pbelevich approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ciflow/periodic Trigger jobs ran periodically on master (periodic.yml) on the PR ciflow/trunk Trigger trunk jobs on your pull request cla signed release notes: profiler release notes category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@robieta @facebook-github-bot @pbelevich