Skip to content

[Profiler][Minor] Organize collection.h/.cpp#82992

Closed
robieta wants to merge 13 commits intogh/robieta/95/basefrom
gh/robieta/95/head
Closed

[Profiler][Minor] Organize collection.h/.cpp#82992
robieta wants to merge 13 commits intogh/robieta/95/basefrom
gh/robieta/95/head

Conversation

@robieta
Copy link
Contributor

@robieta robieta commented Aug 8, 2022 •
edited
Loading

Stack from ghstack (oldest at bottom):

Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

Differential Revision: D38426344

[Profiler][Minor] Organize collection.h/.cpp
1d14cc7 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[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 4b6a9c9 (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:54:15.0663651Z ##[error]The operation was canceled. 
2022-08-20T01:59:39.0797447Z     tasks.join()
2022-08-20T01:59:39.0797905Z   File "/opt/conda/lib/python3.7/site-packages/sphinx/util/parallel.py", line 93, in join
2022-08-20T01:59:39.0798241Z     if not self._join_one():
2022-08-20T01:59:39.0798735Z   File "/opt/conda/lib/python3.7/site-packages/sphinx/util/parallel.py", line 114, in _join_one
2022-08-20T01:59:39.0799116Z     raise SphinxParallelError(*result)
2022-08-20T01:59:39.0799518Z sphinx.errors.SphinxParallelError: ConnectionRefusedError: [Errno 111] Connection refused
2022-08-20T01:59:39.0799870Z 
2022-08-20T01:59:39.0799994Z Sphinx parallel build error:
2022-08-20T01:59:39.0800332Z ConnectionRefusedError: [Errno 111] Connection refused
2022-08-20T01:59:41.4839399Z make: *** [Makefile:42: html] Error 2
2022-08-20T07:54:15.0663651Z ##[error]The operation was canceled.
2022-08-20T07:54:15.0684486Z Prepare all required actions
2022-08-20T07:54:15.0700690Z ##[group]Run ./.github/actions/chown-workspace
2022-08-20T07:54:15.0700905Z ##[endgroup]
2022-08-20T07:54:15.0732893Z ##[group]Run docker run --rm -v "$(pwd)":/v -w /v "${ALPINE_IMAGE}" chown -R "$(id -u):$(id -g)" .
2022-08-20T07:54:15.0733235Z �[36;1mdocker run --rm -v "$(pwd)":/v -w /v "${ALPINE_IMAGE}" chown -R "$(id -u):$(id -g)" .�[0m
2022-08-20T07:54:15.0744672Z shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
2022-08-20T07:54:15.0744889Z env:
2022-08-20T07:54:15.0745122Z   ALPINE_IMAGE: 308535385114.dkr.ecr.us-east-1.amazonaws.com/tool/alpine
2022-08-20T07:54:15.0745349Z ##[endgroup]
2022-08-20T07:54:15.0983676Z Unable to find image '308535385114.dkr.ecr.us-east-1.amazonaws.com/tool/alpine:latest' locally
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
Taylor Robie added 2 commits August 8, 2022 11:57
Update on "[Profiler][Minor] Organize collection.h/.cpp"
dadbc33 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
Update on "[Profiler][Minor] Organize collection.h/.cpp"
335762d 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
robieta pushed a commit that referenced this pull request Aug 8, 2022
[Profiler][Minor] Organize collection.h/.cpp
d84fb1b 
Pull Request resolved: #82992

Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.
ghstack-source-id: 163840418

Differential Revision: [D38426344](https://our.internmc.facebook.com/intern/diff/D38426344/)
@robieta robieta requested a review from chaekit August 8, 2022 21:32
@robieta robieta mentioned this pull request Aug 8, 2022
Closed
pytorchmergebot pushed a commit that referenced this pull request Aug 8, 2022
@pytorchmergebot
[Profiler][Minor] Organize collection.h/.cpp (#82992)
150c443 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

Differential Revision: [D38426344](https://our.internmc.facebook.com/intern/diff/D38426344/)
Pull Request resolved: #82992
Approved by: https://github.com/chaekit
Taylor Robie added 4 commits August 9, 2022 13:07
Update on "[Profiler][Minor] Organize collection.h/.cpp"
ec0b47e 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
Update on "[Profiler][Minor] Organize collection.h/.cpp"
f9db848 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
Update on "[Profiler][Minor] Organize collection.h/.cpp"
bf4797d 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
Update on "[Profiler][Minor] Organize collection.h/.cpp"
5d779b2 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
This was referenced Aug 17, 2022
Closed
Closed
Taylor Robie added 6 commits August 17, 2022 12:55
Update on "[Profiler][Minor] Organize collection.h/.cpp"
36af27f 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
Update on "[Profiler][Minor] Organize collection.h/.cpp"
30437f2 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
Update on "[Profiler][Minor] Organize collection.h/.cpp"
93c890c 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
Update on "[Profiler][Minor] Organize collection.h/.cpp"
3b61a24 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
Update on "[Profiler][Minor] Organize collection.h/.cpp"
d5cbd58 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[ghstack-poisoned]
Update on "[Profiler][Minor] Organize collection.h/.cpp"
4b6a9c9 
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

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

[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][Minor] Organize collection.h/.cpp (#82992) (#82992)
ac47e39 
Summary:
Collection of Torch ops is quite complex compared to backend events / allocations / ooms. Python is also complex, however it is already factored into a standalone unit. This PR just shuffles the contents of collection.cpp to group the Torch op specific parts together, and does various cleanups to the code.

Pull Request resolved: #82992
Approved by: https://github.com/chaekit

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

Original Phabricator Test Plan:
$ PROFILER_SOFT_ASSERT_RAISES=1 TORCH_SHOW_CPP_STACKTRACES=1 buck run mode/opt //caffe2/test:profiler

Reviewed By: chaekit, seemethere

Differential Revision: D38426344

Pulled By: robieta

fbshipit-source-id: ae7e4ffed0570fb7d02dd9cc822732a552d51e35
@facebook-github-bot facebook-github-bot deleted the gh/robieta/95/head branch August 24, 2022 14:20
zejun-chen
zejun-chen reviewed Aug 14, 2024
View reviewed changes
torch/csrc/profiler/collection.cpp
event->allow_tf32_cublas_};

out.emplace_back(Result::create(
time_converter(event->start_time_), tid, kineto_info, std::move(e)));
Copy link
Contributor

@zejun-chen zejun-chen Aug 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hi, @robieta

Here i find the time_converter is always modifying the start time value for the each event of torch ops.
If the torch op's time stamp is getting from __rdtsc() intrinsic, the converter is needed to convert the time stamp value to time value, but if the time stamp is getting from the getTime(), why do the converter is needed here?

inline auto getApproximateTime() {
#if defined(C10_RDTSC)
  return static_cast<uint64_t>(__rdtsc());
#else
  return getTime();
#endif
}

Thank you.

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

Reviewers

@chaekit chaekit chaekit approved these changes

+1 more reviewer

@zejun-chen zejun-chen zejun-chen left review comments

Reviewers whose approvals may not affect merge requirements

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 @chaekit @zejun-chen