-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Remove boost::variant #43100
Remove boost::variant #43100
Conversation
649f3d3
to
d807b7a
Compare
Sorry to inform you that 0f14721's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually. |
836775e
to
23f4a3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for eager_generator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR types
Others
PR changes
Others
Describe
为提升Paddle编译速度,尝试移除Paddle中老旧代码对boost::variant的使用,全量替换为paddle::variant,同时替换variant相关接口。
在全量替换过程中,暴露并在本PR中解决了以下问题:
该问题较棘手:
(1)按照开发者本地源码编译方式安装Paddle,无法复现此问题
(2)同样的测试机器按照Windows CI的环境部署和脚本运行,可复现错误
(3)出错位置代码语法没有明显问题,也捕捉不到代码层面的任何异常,怀疑是程序运行到函数返回处时被操作系统杀死,可能是底层存在一些bug导致此处函数返回后跳转地址错误,访问到非法地址而被OS拦截
(4)bug可能与requirement中的第三方依赖库有关(执行CI脚本后相同机器从稳定无法复现变成稳定复现,CI脚本运行后能遗留下来污染环境的只有pip第三方库的更新,但目前已难以回退到之前可以不出问题的环境);也可能是paddle的variant在嵌套使用时有问题(出问题的地方是嵌套variant,框架中别处都没有嵌套使用)
(5)对此问题目前没有太好的排查思路,因而对PE的FetchResultType先不做variant替换,后续重写PE相关函数接口,改成不用variant存储fetch_data,相关位置的get调用也临时性地使用裸的boost::get(此PR合入后提新PR修改PE并进行移除)
本PR后续TODO: