use PYTHON_C_API in dygraph #32524

wanghuancoder · 2021-04-25T06:44:28Z

PR types

Performance optimization

PR changes

Others

Describe

动态图对外暴露core.ops不再使用pybind11，改用Python_c_API。原因是Python_c_API的性能要更好一些，以降低动态图的调度成本。
本PR的修改的一些关键点：

使用op_function_generator.cc生成代码的“框架”，微调生成代码的格式、内容。
本次只修改了动态图API，使用Python_c_API。其余部分仍使用pybind11。
- 在使用Python_c_API时，需将动态图API通过pybind11声明的core.ops向外暴露。
- VarBase向Python的暴露仍是有pybind11完成的。因此，VarBase的输入输出需做特殊处理。
由于用户传入的attr有各种类型，新的代码必须均能支持，如numpy.int64、numpy.ndarray、Tensor等特殊类型，也需支持
报错信息，原本是在文件exception.cc中，通过pybind11将异常抛到python端的，因此Python_c_API也要有能力捕获异常、向python抛出异常

其中自动生成的op_function_impl.h代码请见：
https://gist.github.com/wanghuancoder/65be9f6d527e7da036f9ff5bc347c9bc

经测试，动态图性能略有提升：

对比项目	develop	develop + Python_C_API	提升比例
core.ops.elementwise_add	184.046 s	137.275 s	25.4%
Linear+relu(GPU)	57.824 s	52.177 s	9.8%
Linear+relu(CPU)	64.545 s	59.769 s	7.4%
MaskRCNN_BS4_FP16	9.770 img/s	10.096 img/s	3.3%
PTB small	496.863 words/s	535.241 words/s	7.7%
Pahelix	237.306 s	235.473 s	0.8%

core.ops.elementwise_add耗时点对比：

耗时点	pybind11	Python_C_API
VarBase解析	4,757,120	2,031,758
Attr解析	38,639,099	7,841,931
GetCurrentTracer	2,391,581	1,569,099
Out map构造	16,708,953	14,194,468
in map构造	7,118,836	7,032,639
TraceOp	89,742,151	78,438,589
返回值打包		9,546,960
python交互	40,413,613	25,644,535

paddle-bot-old · 2021-04-25T06:44:30Z

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

… pythoncapi

paddle-bot-old · 2021-05-06T02:35:52Z

Sorry to inform you that 090a540's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

… pythoncapi

chenwhql · 2021-05-13T13:13:48Z

paddle/fluid/pybind/op_function_generator.cc

 const char* IN_VAR_LIST_TYPE = R"(py::handle)";

 const char* OUT_VAR_TYPE = R"(std::shared_ptr<imperative::VarBase>)";
 const char* OUT_VAR_LIST_TYPE = R"(std::vector<std::shared_ptr<imperative::VarBase>>)";


在这个文件上面，output创建这个地方，貌似用make_shared更快一些，可以试一试

const char* OUT_INITIALIZER_TEMPLATE = R"({"%s", {std::shared_ptr<imperative::VarBase>(new imperative::VarBase(tracer->GenerateUniqueName()))}})";

https://stackoverflow.com/questions/20895648/difference-in-make-shared-and-normal-shared-ptr-in-c

我改完，使用core.ops.elementwise_add测了一下，没有发现明显收益。但既然理论上make_shared更优，应当在一些地方还是有收益的。只是在core.ops.elementwise_add这种极简单的测试场景下体现不出来。
现在已经改成了使用make_shared。

chenwhql · 2021-05-14T02:08:37Z

paddle/fluid/pybind/op_function.h

 namespace paddle {
 namespace pybind {

+std::unordered_map<


编程规范里有这样一条，禁止定义静态生存周期的对象，这个地方建议用单例模式管理，以明确对象的初始化顺序

 https://zh-google-styleguide.readthedocs.io/en/latest/google-cpp-styleguide/scoping/#static-and-global-variables

chenwhql · 2021-05-14T02:10:35Z

paddle/fluid/pybind/op_function.h

+        if (PyObject_CheckLong(obj)) {
+          attrs[key] = (int)PyLong_AsLong(obj);  // NOLINT
+        } else {
+          PADDLE_THROW(platform::errors::InvalidArgument(


这个THROW语句都是重复的，要不要写到一个地方统一调用，之后如果要完善报错信息会好维护一些

是有些THROW是重复的，但有些地方有细微的差异。另外，抛出异常的行数是比较重要的调试信息。如果放到统一的地方THROW，行数信息就丢掉了。我保持了原样。

也可以写成宏，可以保留行号。

chenwhql · 2021-05-14T02:17:18Z

paddle/fluid/pybind/op_function.h

+  return result;
+}
+
+void init_ops_attrtype_map() {


建议函数命名风格统一，这里也使用驼峰式命名

done，thx！

chenwhql · 2021-05-14T02:17:24Z

paddle/fluid/pybind/op_function.h

+PyObject* EnforceNotMetException =
+    PyErr_NewException("paddle.EnforceNotMet", PyExc_Exception, NULL);
+
+void throw_exception_to_python(std::exception_ptr p) {


done，thx！

zhiqiu · 2021-05-17T11:28:19Z

paddle/fluid/pybind/imperative.cc

 namespace paddle {
 namespace pybind {

+PyTypeObject *g_VarBase_PyType = NULL;


Better use either camelcase(variableName) or underscores(variable_name), not combined.

done，thx！

zhiqiu · 2021-05-17T11:32:02Z

paddle/fluid/pybind/op_function.h

+#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
+#define PY_ARRAY_UNIQUE_SYMBOL MY_PyArray_API


Does it seem no usage of these marcos?

这个是为了解决include numpy的头文件导致的编译错误。

zhiqiu · 2021-05-17T11:32:55Z

paddle/fluid/pybind/op_function.h

+#define INIT_NUMPY_ARRAY_CPP
+#ifndef INIT_NUMPY_ARRAY_CPP
+#define NO_IMPORT_ARRAY  // for usual translation units
+#endif


Why need this macro?

不必要的已经删除

zhiqiu · 2021-05-17T11:33:37Z

paddle/fluid/pybind/op_function.h

 #include "paddle/fluid/imperative/type_defs.h"
 #include "paddle/fluid/pybind/imperative.h"
+#pragma GCC diagnostic ignored "-Wconversion-null"
+#pragma GCC diagnostic ignored "-Wunused-variable"


Why ignore warning?

最早有些实现方式需要用到，现在已经不需要了，已经删除。

zhiqiu · 2021-05-17T11:35:30Z

paddle/fluid/pybind/op_function.h

+int init_numpy() {
+  import_array();
+  return 0;
+}
+const static int numpy_initialized = init_numpy();  // NOLINT


I suggest add some comments on this, it is not easy to follow.

最早有些实现方式需要用到，现在已经不需要了，已经删除。

zhiqiu · 2021-05-17T11:41:46Z

paddle/fluid/pybind/op_function.h

+}
+
+inline bool PyObject_CheckFloat(PyObject* obj) {
+  return PyFloat_Check(obj) || PyLong_Check(obj) ||


Better comment on why PyLong_Check is added.

done，thx！

zhiqiu · 2021-05-17T11:45:21Z

paddle/fluid/pybind/op_function.h

+  PADDLE_ENFORCE_EQ(
+      (attr_end - attr_start + 1) % 2, 0,
+      platform::errors::InvalidArgument(
+          "The number of arguments for arributes should be even."));


typo: arributes -> attributes

done，thx！

zhiqiu · 2021-05-17T11:49:40Z

paddle/fluid/pybind/op_function.h

+    const std::string& op_type, PyObject* args, ssize_t attr_start,
+    ssize_t attr_end, paddle::framework::AttributeMap& attrs) {  // NOLINT
+  PADDLE_ENFORCE_EQ(
+      (attr_end - attr_start + 1) % 2, 0,


Not important. I suggest passing attr_end+1 directly to avoid some problems, [start, end) is commonly used (which means start is reachable and end is not).

done，thx！

zhiqiu · 2021-05-17T11:49:56Z

paddle/fluid/pybind/op_function.h

+  auto attr_type_map = &(OpAttrTypeMap::Instance().Map()[op_type]);
+
+  PyObject* obj = nullptr;
+  for (ssize_t arg_pos = attr_start; arg_pos < attr_end + 1; arg_pos += 2) {


Same for attr_end.

done，thx！

zhiqiu · 2021-05-17T11:50:32Z

paddle/fluid/pybind/op_function_generator.cc

-    py::gil_scoped_release release;
+    %s
+    framework::AttributeMap attrs;
+    ConstructAttrMapFromPyArgs("%s", args, %d, PyTuple_GET_SIZE(args)-1 , attrs);


Better pass PyTuple_GET_SIZE(args).

done，thx！

zhiqiu · 2021-05-17T11:56:59Z

paddle/fluid/pybind/op_function.h

+    const char* key_prt;
+    obj = PyTuple_GET_ITEM(args, arg_pos);
+    if (PyObject_CheckString(obj)) {
+      key_prt = PyUnicode_AsUTF8AndSize(obj, &key_len);


Not sure, key_prt -> key_ptr ?

done，thx！

zhiqiu · 2021-05-17T12:01:01Z

paddle/fluid/pybind/op_function.h

+        if (PyObject_CheckLong(obj)) {
+          attrs[key] = (int)PyLong_AsLong(obj);  // NOLINT
+        } else {
+          PADDLE_THROW(platform::errors::InvalidArgument(


也可以写成宏，可以保留行号。

zhiqiu · 2021-05-17T12:04:09Z

paddle/fluid/pybind/op_function.h

+        break;
+    }
+  }
+}


Suggest adding a function for each case to make this function short.

已经为每个case创建了函数，PADDLE_THROW不仅是行号的问题，认真看报错内容的话，不同的PADDLE_THROW抛出的异常内容、格式略有差别，不适合封装公共异常抛出代码。

zhiqiu · 2021-05-20T03:11:15Z

paddle/fluid/pybind/op_function.h

+            op_type, arg_name, arg_idx,
+            ((PyTypeObject*)((PyObject*)item)->ob_type)->tp_name));  // NOLINT
+      }
+      void** vh = item->simple_layout ? item->simple_value_holder


Better add comments on simple_layout?

zhiqiu · 2021-05-20T03:12:21Z

paddle/fluid/pybind/op_function.h

+      void** vh = item->simple_layout ? item->simple_value_holder
+                                      : &item->nonsimple.values_and_holders[0];
+      result.emplace_back(
+          reinterpret_cast<std::shared_ptr<paddle::imperative::VarBase>&>(


As discussed offline, what will be happended to the lifetime fo VarBase?

… pythoncapi

jzhang533

instead of using NULL macro, using nullptr would be better.

wanghuancoder · 2021-06-11T03:11:43Z

instead of using NULL macro, using nullptr would be better.

done.thx!

use PYTHON_C_API in dygraph, test=develop

1582a36

wanghuancoder added 6 commits April 25, 2021 07:14

modify return type, test=develop

46ce8b9

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

32b597b

… pythoncapi

fix test case error, test=develop

2caa501

attr map, test=develop

dd3c657

fix numpy, test=develop

a760e4c

fix throw exception bug, test=develop

090a540

wanghuancoder added 14 commits May 7, 2021 03:25

fix convert python var to varbase, test=develop

372a1ff

fix some testcase fail, test=develop

e75f09e

modify gil lock, test=develop

046f117

modify gil lock, test=develop

96314f8

numpy64 error, test=develop

c149b38

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

0024132

… pythoncapi

return None, test=develop

36a3f7f

cache return NULL, test=develop

e0df6e4

numpy64, test=develop

bea4ea6

refine, test=develop

690068f

numpy, test=develop

6878ff5

numpy, test=develop

c104201

numpy.ndarray, test=develop

c9c143e

Tensor, test=develop

cec78dc

chenwhql reviewed May 13, 2021

View reviewed changes

chenwhql reviewed May 14, 2021

View reviewed changes

refine, test=develop

89be180

zhiqiu reviewed May 17, 2021

View reviewed changes

wanghuancoder added 2 commits May 17, 2021 12:18

refine, test=develop

49df08a

refine, test=develop

cb895dd

wanghuancoder added 2 commits May 18, 2021 04:42

refine, test=develop

ac77ef6

refine, test=develop

198a8a1

zhiqiu reviewed May 20, 2021

View reviewed changes

wanghuancoder and others added 8 commits May 25, 2021 05:22

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

0fe1ab9

… pythoncapi

modify convert_shape_to_list when convert tensor to list, test=develop

81034c6

refine convert bool, test=develop

e5ed284

solve numpy version problem, test=develop

6e1fab1

refine numpy convert, test=develop

9f9127c

refine, test=develop

599cd93

Merge branch 'develop' into pythoncapi

c47b6f3

convert blockdesc, test=develop

d968e63

jzhang533 reviewed Jun 11, 2021

View reviewed changes

NULL to nullptr, test=develop

247d251

refine, test=develop

999a1c6

phlrain self-requested a review June 11, 2021 06:43

phlrain approved these changes Jun 11, 2021

View reviewed changes

wanghuancoder merged commit 08e8147 into PaddlePaddle:develop Jun 11, 2021

This was referenced Jun 17, 2021

fix import paddle error in windows for python3.8 and python3.9 #33617

Merged

fix bug about deallocating None #33723

Merged

		#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
		#define PY_ARRAY_UNIQUE_SYMBOL MY_PyArray_API

use PYTHON_C_API in dygraph #32524

use PYTHON_C_API in dygraph #32524

Uh oh!

Conversation

wanghuancoder commented Apr 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR types

PR changes

Describe

Uh oh!

paddle-bot-old bot commented Apr 25, 2021

Uh oh!

paddle-bot-old bot commented May 6, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chenwhql May 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wanghuancoder commented Apr 25, 2021 •

edited

Loading

chenwhql May 14, 2021 •

edited

Loading