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 force link macro #1054

Closed

Conversation

reyoung
Copy link
Collaborator

@reyoung reyoung commented Jan 3, 2017

  • It could help us get rid of --whole-archive.

@reyoung reyoung requested review from gangliao and hedaoyuan January 3, 2017 07:09
#include <paddle/utils/ForceLink.h>
#include "test_ClassRegistrarLib.h"
// Enable link test_ClassRegistrarLib.cpp
PADDLE_ENABLE_FORCE_LINK_FILE(test_registrar);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

instead of use --whole-archive, we could use PADDLE_ENABLE_FORCE_LINK_FILE macro to force link XXXLayer.cpp in TrainerMain.

It also could help us for predict SDK. We can generate PADDLE_ENABLE_FORCE_LINK_FILE for XXXPredict, which only contains the Layers that Predict binary used.

@reyoung
Copy link
Collaborator Author

reyoung commented Jan 3, 2017

C/C++编译器在静态连接的时候,会初始化使用到符号的文件的所有静态变量。所以,我们在需要初始化的CPP文件里面定义一个没啥意义的函数,在使用这个文件的地方,使用这个没啥意义的函数,就能够初始化这个CPP文件里面的所有静态变量了。

https://github.com/PaddlePaddle/Paddle/pull/1054/files#diff-6219ecc16db60fe517ce956bb9083561R17

* It could help us get rid of --whole-archieve.
* Add comments
@reyoung reyoung force-pushed the feature/remove_whole_archieve branch from a6677d8 to 7848214 Compare January 3, 2017 09:10
@reyoung
Copy link
Collaborator Author

reyoung commented Jan 3, 2017

@gangliao @hedaoyuan Remove ' --whole-archive', instead, we add definitions to force linking files here

@gangliao
Copy link
Contributor

gangliao commented Jan 3, 2017

@reyoung

  1. paddle_ld_flags.py
  2. paddle/setup.py.in

* this file used for static linking libraries, to enable all InitFunction in
* Paddle GServer.
*/
PADDLE_ENABLE_FORCE_LINK_FILE(base_data_providers);
Copy link
Contributor

@hedaoyuan hedaoyuan Jan 3, 2017

Choose a reason for hiding this comment

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

写个函数(可以赋给一个静态的函数指针变量),里面调用一下每个Layer的构造函数因该就可以了,没必每个Layer文件都新增一个符号。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这个新增一个符号要比直接引用那个Layer的构造函数耦合度要松一些。

1、这个新增的符号命名使用配对的宏去做的,所以不需要include头文件。
2、这个新增的符号是根据『文件』这个级别走的。也就是link某个『文件』的全部static变量,而不是某个『Layer』。这样做的好处是:

  • Layer可以改名字。新增Layer而不新增文件,不需要修改这个ForceLinkFiles.cpp
  • 一组Layer的REGISTER部分可以放到一个文件里,这样也不需要写很多PADDLE_ENABLE_FORCE_LINK_FILE
    • 譬如 写一个 cnn_register.cpp,里面全是图像相关的Layer的REGISTER,那我们写一个PADDLE_ENABLE_FORCE_LINK_FILE(cnn_register)就好了。

Copy link
Contributor

Choose a reason for hiding this comment

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

这个新增一个符号要比直接引用那个Layer的构造函数耦合度要松一些

这个都是一样,都是在另外一个.o中引用一下某个Layer.o中的符号

Layer可以改名字。新增Layer而不新增文件,不需要修改这个ForceLinkFiles.cpp

Layer的名字一般不会修改,一般Layer都是单独写一个文件的,即使要在同一个文件中新增一个Layer,不修改ForceLinkFiles.cpp也没什么问题(同样对于一个全是图像Layer的文件,只引用其中一个Layer的构造函数并没什么问题)。

本质上确实不需要增加额外的符号来实现--whole-archive;对外,提供.so 不存在--whole-archive问题;提供.a给别人用,是否需要--whole-archive以及不同编译环境在该怎么实现--whole-archive这个是自己决定的。即使现在这种编译方式,在ar的时候去掉-s好像就不需要--whole-archive了。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

1、这么搞不用include另一个头文件了呀。。否则,那个ForceLinkFiles.cpp前面会有一堆include的。
2、况且,有的东西可以没有头文件的。比如PyDataProvider2.cpp

本质上确实不需要增加额外的符号来实现--whole-archive

增加这些符号只是为了写起来方便。确实可以不增加符号。但增加了这些符号也没啥坏处。

Copy link
Collaborator

@wangkuiyi wangkuiyi left a comment

Choose a reason for hiding this comment

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

--whole-archive 有什么不好?为什么需要在源码级别引入一些symbols来实现force linking呢?

@reyoung
Copy link
Collaborator Author

reyoung commented Jan 4, 2017

--whole-archive 有什么不好?为什么需要在源码级别引入一些symbols来实现force linking呢?

whole-archive有以下几点问题:

  • 不是所有平台都支持--whole-archive的链接选项的,这个是GCC特有的。虽然clang也可以模拟这件事情。但是写起来逻辑比较复杂。
  • 去掉--whole-archive可以简化cmake的很多连接选项。
    • 这降低了理解Paddle链接过程的心智负担。让Paddle的链接逻辑可以变得更简单。
      • 这点,其实也和当时要用bazel替换cmake的初衷一致。最好让Paddle的链接只有-lpaddle_math -lpaddle_utils ...这样的最简单的链接选项。
    • 特别是,如果我们要expose多语言接口,那么Paddle可能会编译成一个.so或者.a给别人用。如果用户只用一个-lpaddle那是最方便的。而要用 --whole-archive -lpaddle --no-whole-archive对于用户也比较麻烦。
  • 手动写一些symbols来实现force linking。一方面这个事情可以实现whole-archive的全部功能。同时也有一些其他的side-effect
    • 使用手写需要链接哪些symbols的方法,我们可以编译一个Paddle预测库,只包含某些Layer的。
      • 例如对于图像预测库的需求,我们只需要编译CNN相关的Layer就好了。可以减小Paddle二进制的尺寸。

@reyoung
Copy link
Collaborator Author

reyoung commented Jan 4, 2017

paddle_ld_flags.py
paddle/setup.py.in

有道理,我改一下。。

@reyoung
Copy link
Collaborator Author

reyoung commented Jan 4, 2017

Closed.

线下和 @hedaoyuan 讨论。这块的工作可以分为一下几步进行。

  • 将libpaddle_gserver.a 拆解成若干子library

    • libpaddle_gradient_machine.a
    • libpaddle_layers.a
    • libpaddle_evaluator.a
    • libpaddle_data_providers.a
  • 对需要whole archive的libraries,调用ar的时候,去掉ar -s

    • libpaddle_layers.a
    • libpaddle_evaluator.a
    • libpaddle_data_providers.a
  • 至此可以去掉--whole-archive

@reyoung reyoung closed this Jan 4, 2017
zhhsplendid pushed a commit to zhhsplendid/Paddle that referenced this pull request Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants