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.

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

Add CPU time and MemCopy to the timeline. #8679

Merged
merged 2 commits into from
Mar 2, 2018

Conversation

panyx0718
Copy link
Contributor

No description provided.

@panyx0718 panyx0718 changed the title Add CPU time to the timeline. Add CPU time and MemCopy to the timeline. Mar 1, 2018
std::string MemcpyKind(CUpti_ActivityMemcpyKind kind) {
switch (kind) {
case CUPTI_ACTIVITY_MEMCPY_KIND_HTOD:
return "MEMCPYHtoD";
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just HtoD is ok? There are many duplicates MEMCPY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I Added it back. User can search MEMCPY to find all copy.

@@ -15,12 +15,17 @@ limitations under the License. */
syntax = "proto2";
package paddle.platform.proto;

message MemCopy { optional uint64 bytes = 3; }
Copy link
Member

Choose a reason for hiding this comment

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

why start from 3?

event->set_start_ns(r.start_ns);
event->set_end_ns(r.end_ns);
event->set_stream_id(r.thread_id);
event->set_device_id(-1);
Copy link
Member

Choose a reason for hiding this comment

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

use a const value to represent CPU instead of -1, like

const kCpuDeviceId = -1;

Copy link
Member

@jacquesqiao jacquesqiao left a comment

Choose a reason for hiding this comment

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

LGTM

@panyx0718 panyx0718 merged commit 92974d4 into PaddlePaddle:develop Mar 2, 2018
@reyoung reyoung added this to Doing in Performance Tuning Mar 12, 2018
@reyoung reyoung moved this from Doing to Done in Performance Tuning Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants