Skip to content

Conversation

@rainyfly
Copy link
Contributor

@rainyfly rainyfly commented Mar 2, 2022

PR types

New features

PR changes

APIS

Describe

  1. pybind绑定C++ Profiler有关接口到python层
  2. Python层新增profiler package
@paddle-bot-old
Copy link

paddle-bot-old bot commented Mar 2, 2022

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.


py::class_<paddle::platform::ProfilerOptions>(m, "ProfilerOptions")
.def(py::init<>())
.def_readwrite("trace_level",
Copy link
Contributor

Choose a reason for hiding this comment

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

这个level暴漏吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已改

profiler->Prepare();
})
.def("Start", &paddle::platform::Profiler::Start)
.def("Stop", &paddle::platform::Profiler::Stop,
Copy link
Contributor

Choose a reason for hiding this comment

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

stop时disable HostEventRecorder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已添加

Copy link
Contributor

@liutiexing liutiexing left a comment

Choose a reason for hiding this comment

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

LGTM

.def_readwrite("device_node",
&paddle::platform::HostPythonNode::device_node_ptrs);

py::class_<paddle::platform::Profiler>(m, "_Profiler")
Copy link
Contributor

Choose a reason for hiding this comment

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

这块的Profiler相关调用函数首字母大写,这块是有调研过竞品,都是大写?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改为小写


py::class_<platform::RecordEvent>(m, "_RecordEvent")
.def(py::init([](std::string name, platform::TracerEventType type) {
return std::unique_ptr<platform::RecordEvent>(new platform::RecordEvent(
Copy link
Contributor

Choose a reason for hiding this comment

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

这里使用make_unique的性能是不是更好一点?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

import paddle.profiler as profiler
with profiler.Profiler(targets=[profiler.ProfilerTarget.CPU,
profiler.ProfilerTarget.GPU],
scheduler = (3, 10),
Copy link
Contributor

Choose a reason for hiding this comment

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

这里有个疑问,虽然在构建scheduler的时候可以多种方式,但是给用户的感觉就是构建方法不够清晰,这里是不是考虑把接口统一一下

def __exit__(self, exc_type, exc_val, exc_tb):
self.stop()

def start(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

start和stop的文档补齐一下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已补充

Copy link
Contributor

@wawltor wawltor left a comment

Choose a reason for hiding this comment

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

LGTM

@wawltor wawltor merged commit 10325a8 into PaddlePaddle:develop Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants