WIP feature/Inference Analysis booted with TensorRT engine #10236
Conversation
wangkuiyi
left a comment
wangkuiyi
left a comment
There was a problem hiding this comment.
谢谢 @Superjomn 的这个PR。我对这个base class的设计有些不解之处。请看看我的comment吧。
| * TensorRT's ITensor follows row major, NCHW. Fluid is also row major, so in | ||
| * most cases just need to copy the data. | ||
| */ | ||
| class EngineInputConverter { |
There was a problem hiding this comment.
TensorRT engine 或者 Anajin engine, 用于sub-block的加速
| } // namespace inference | ||
| } // namespace paddle | ||
|
|
||
| #define REGISTER_TRT_INPUT_CONVERTER(in_op_type__, Converter__) \ |
| (*it->second)(in, out, max_size); | ||
| } | ||
|
|
||
| static EngineInputConverter& Global() { |
There was a problem hiding this comment.
应该返回一个 const reference 或者一个pointer
| public: | ||
| EngineInputConverter() {} | ||
|
|
||
| virtual void operator()(const LoDTensor& in, void* out, size_t max_size) {} |
There was a problem hiding this comment.
这里 max_size 的语义是什么呢？是说如果input tensor太大了，要截断？还是放弃conversion？
| EngineInputConverter() {} | ||
|
|
||
| virtual void operator()(const LoDTensor& in, void* out, size_t max_size) {} | ||
| void Execute(const std::string& in_op_type, const LoDTensor& in, void* out, |
There was a problem hiding this comment.
既然class name是xxxConverter，我理解 operator() 回去执行 convert。顾名思义，Execute也应该是执行 conversion 的。但是这样一来，operator() 和 Executor 的意思就重复了？
There was a problem hiding this comment.
的确，Converter.Run 可能好一些
| */ | ||
| class EngineInputConverter { | ||
| public: | ||
| EngineInputConverter() {} |
There was a problem hiding this comment.
这个 Converter 是用来 convert 什么的呢？从 operator() 看貌似是convert LoDTensor的。其他Fluid类型是否需要convert？如果要，那么应该用什么method来convert呢？
Executor 看上去在执行一些operators。是说这个 converter 也需要能convert operators吗？如果是，那么是不是应该把 operator() 改名成 ConvertVar，把 Execute 改名成 ConvertOp？
There was a problem hiding this comment.
Convert Engine 的 Input
比如数据流是
fluid op -> LoDTensor -> ITensor -> TensorRT engine
这里的 EngineInputConverter 完成 LoDTensor.data() -> ITensor.data 的数据转换，考虑到无法保证 每个FluidOp 的输出都能用相同的方式转换成TensorRT的输入，所以用工厂为每个相关的Fluid Op 留下定制转换过程的空间。
| cudaStream_t* stream_{nullptr}; | ||
|
|
||
| private: | ||
| std::unordered_map<std::string, |
There was a problem hiding this comment.
这样一个class里，实现了多个 design patterns：(1) singleton，（2）registration，（3）operator overloading。这几个都是 code style 里提醒需要慎用的：
- https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables
- https://google.github.io/styleguide/cppguide.html#Operator_Overloading
- https://google.github.io/styleguide/cppguide.html#Preprocessor_Macros
我们真的需要他们吗？
| @@ -0,0 +1,55 @@ | |||
| /* Copyright (c) 2018 PaddlePaddle Authors. All Rights Reserved. | |||
There was a problem hiding this comment.
io_converter.cc和io_converter.h可以放到convert文件夹下么？
| } // namespace inference | ||
| } // namespace paddle | ||
|
|
||
| #define REGISTER_TRT_INPUT_CONVERTER(in_op_type__, Converter__) \ |
| private: | ||
| std::unordered_map<std::string, | ||
| ::paddle::inference::tensorrt::EngineInputConverter*> | ||
| converters_; |
There was a problem hiding this comment.
::paddle::inference::tensorrt::EngineInputConverter* -》 EngineInputConverter，更加简短。
| #define REGISTER_TRT_INPUT_CONVERTER(in_op_type__, Converter__) \ | ||
| struct trt_input_##in_op_type__##_converter { \ | ||
| trt_input_##in_op_type__##_converter() { \ | ||
| ::paddle::inference::tensorrt::EngineInputConverter::Global() \ |
There was a problem hiding this comment.
::paddle::inference::tensorrt::EngineInputConverter* -》 EngineInputConverter，更加简短。
Superjomn
force-pushed
the
feature/convert_trt_io
branch
from
da930ab to
729c703
Compare
Superjomn
changed the title
Superjomn
changed the title
|
这个PR里面的内容太多了，图相关的内容可以放在另一个PR里面吧，尽量使每个PR的功能相对独立且简单。 |
4 participants
No description provided.