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.

Sign up for GitHub

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

support build with Ninja on Linux #44210

Merged
merged 11 commits into from
Aug 1, 2022
Merged

support build with Ninja on Linux #44210

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7e8ba9a
support ninja
gglin001 Jul 11, 2022
f24ec6e
fix mkldnn on windows
gglin001 Jul 11, 2022
644ce58
fix mkldnn on windows up1
gglin001 Jul 11, 2022
f6f206f
up2
gglin001 Jul 11, 2022
0b34029
up3
gglin001 Jul 11, 2022
c7dedea
fix gflags
gglin001 Jul 14, 2022
473c2a1
Merge branch 'develop' into support_build_with_ninja
gglin001 Jul 14, 2022
d55837e
BUILD_BYPRODUCTS_OPTION -> BUILD_BYPRODUCTS_ARGS
gglin001 Jul 14, 2022
d689d16
use CMAKE_COMMAND
gglin001 Jul 14, 2022
0136c32
Merge branch 'develop' into support_build_with_ninja
gglin001 Jul 22, 2022
fa5b56c
up x
gglin001 Jul 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmake/external/dgc.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ ExternalProject_Add(
URL_MD5 "94e6fa1bc97169d0e1aad44570fe3251"
PREFIX "${DGC_PREFIX_DIR}"
CONFIGURE_COMMAND ""
BUILD_COMMAND make -j $(nproc)
BUILD_COMMAND make -j${NPROC}
Copy link
Contributor

Choose a reason for hiding this comment

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

这个make -j 的命令在ninja时是否能编过？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ninja 下是ok的, 因为 ${NPROC} 会被替换成 数字, build.ninja 里面不会出现 $

Copy link
Contributor

Choose a reason for hiding this comment

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

这个编dgc时是会用 make -j${NPROC} 来跑

INSTALL_COMMAND
mkdir -p ${DGC_INSTALL_DIR}/lib/ ${DGC_INCLUDE_DIR}/dgc && cp
${DGC_SOURCES_DIR}/build/lib/libdgc.a ${DGC_LIBRARIES} && cp
Expand Down
4 changes: 2 additions & 2 deletions cmake/external/gflags.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ else()
set(GFLAGS_LIBRARIES
"${GFLAGS_INSTALL_DIR}/lib/libgflags.a"
CACHE FILEPATH "GFLAGS_LIBRARIES" FORCE)
set(BUILD_COMMAND $(MAKE) --silent)
set(INSTALL_COMMAND $(MAKE) install)
set(BUILD_COMMAND ${CMAKE_COMMAND} --build .)
set(INSTALL_COMMAND ${CMAKE_COMMAND} --build . --target install)
endif()

include_directories(${GFLAGS_INCLUDE_DIR})
Expand Down
9 changes: 8 additions & 1 deletion cmake/external/mkldnn.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ else()
CACHE FILEPATH "mkldnn library." FORCE)
endif()

if(UNIX AND NOT APPLE)
Copy link
Contributor

Choose a reason for hiding this comment

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

目前Windows、Linux都支持ninja，这里副产物是不是除了mac都适用？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里是因为 windows 的 CI 有问题, mac 下 我没测试, 所以只在 linux 下设置

详细的log:
https://xly.bce.baidu.com/paddlepaddle/paddle/newipipe/detail/6075099/job/16531188

Copy link
Contributor

Choose a reason for hiding this comment

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

还是改成if(NOT MAC)，然后处理下Windows的情形，因为Windows现在也支持ninja

Copy link
Contributor Author

@gglin001 gglin001 Jul 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

这里修改的细节见下面的回复, 根据 CI 的结果, windows 下不设置 BUILD_BYPRODUCTS 也是可以正常编译的, 反而设置了该变量后会出现问题

可以在后面对 第三方库的 cmake 配置做统一的修改, 实现:

  1. 不区分平台的统一配置(目前有些区分了 win 和 非 win 平台)
  2. 抛弃 make -j 的写法(使用 cmake 的标准配置)
  3. 更加统一规范的写法, 比如对 ExternalProject_Add() 的使用

一些良好的参考, 比如
https://fuchsia.googlesource.com/cobalt/+/refs/tags/v0.1.3/CMakeLists.txt

Copy link
Contributor

Choose a reason for hiding this comment

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

这个写 if(LINUX)吧，标准一些。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

set(BUILD_BYPRODUCTS_ARGS ${MKLDNN_LIB})
else()
set(BUILD_BYPRODUCTS_ARGS "")
endif()

ExternalProject_Add(
${MKLDNN_PROJECT}
${EXTERNAL_PROJECT_LOG_ARGS} ${SHALLOW_CLONE}
Expand All @@ -83,7 +89,8 @@ ExternalProject_Add(
-DCMAKE_POSITION_INDEPENDENT_CODE=ON
-DDNNL_BUILD_TESTS=OFF
-DDNNL_BUILD_EXAMPLES=OFF
CMAKE_CACHE_ARGS -DCMAKE_INSTALL_PREFIX:PATH=${MKLDNN_INSTALL_DIR})
CMAKE_CACHE_ARGS -DCMAKE_INSTALL_PREFIX:PATH=${MKLDNN_INSTALL_DIR}
BUILD_BYPRODUCTS ${BUILD_BYPRODUCTS_ARGS})
Copy link
Contributor

Choose a reason for hiding this comment

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

这个BUILD_BYPRODUCTS是不是就是 ${MKLDNN_LIB}，不用再设置新的变量了

Copy link
Contributor Author

@gglin001 gglin001 Jul 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@gglin001
[fix mkldnn on windows](https://github.com/PaddlePaddle/Paddle/pull/44210/commits/f24ec6e490544b2890346c877be6865270f77e12)
[f24ec6e](https://github.com/PaddlePaddle/Paddle/pull/44210/commits/f24ec6e490544b2890346c877be6865270f77e12)
@gglin001
[fix mkldnn on windows up1](https://github.com/PaddlePaddle/Paddle/pull/44210/commits/644ce589b8d1f53c2a337981227450456bd8f16a)
[644ce58](https://github.com/PaddlePaddle/Paddle/pull/44210/commits/644ce589b8d1f53c2a337981227450456bd8f16a)
@gglin001
[up2](https://github.com/PaddlePaddle/Paddle/pull/44210/commits/f6f206f5aab4da411978bd3627156cc188baa0c8)
[f6f206f](https://github.com/PaddlePaddle/Paddle/pull/44210/commits/f6f206f5aab4da411978bd3627156cc188baa0c8)
@gglin001
[up3](https://github.com/PaddlePaddle/Paddle/pull/44210/commits/0b3402908313bf5f757266673b190c20a8c3e9fa)

可以参考下这几次提交的 CI 失败结果, 这里的 BUILD_BYPRODUCTS 设置成 ${MKLDNN_LIB} 时, windows 的 CI 会失败, 这也是这个修改的原因


message(STATUS "MKLDNN library: ${MKLDNN_LIB}")
add_definitions(-DPADDLE_WITH_MKLDNN)
Expand Down
2 changes: 1 addition & 1 deletion cmake/external/openblas.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ if(NOT WIN32)
PREFIX ${CBLAS_PREFIX_DIR}
INSTALL_DIR ${CBLAS_INSTALL_DIR}
BUILD_IN_SOURCE 1
BUILD_COMMAND make -j$(nproc) ${COMMON_ARGS} ${OPTIONAL_ARGS}
BUILD_COMMAND make -j${NPROC} ${COMMON_ARGS} ${OPTIONAL_ARGS}
Copy link
Contributor

Choose a reason for hiding this comment

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

make 命令 在ninja时能否跑通

Copy link
Contributor Author

Choose a reason for hiding this comment

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

同上, 我这里测试时是 ok的, 这里和一些其他的一些写死使用 make的 三方库可以在后面慢慢更新成 配置使用 cmake 而不是 make

Copy link
Contributor

Choose a reason for hiding this comment

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

这里编openblas，按道理调用了make -j 应该会出错呀

Copy link
Contributor

@tiancaishaonvjituizi tiancaishaonvjituizi Jul 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

为什么会出错呢，这里 paddle 只需要用编译产物，具体编译过程是用 make 还是 ninja 还是裸调用 gcc 都是不关心的

Copy link
Contributor

@zhwesky2010 zhwesky2010 Jul 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

为什么会出错呢，这里 paddle 只需要用编译产物，具体编译过程是用 make 还是 ninja 还是裸调用 gcc 都是不关心的

我是说应该让只安装了ninja 没有安装make的用户能编过，才算完整支持ninja。既然已经设置了CMAKE -GNinja，却还要背后里依赖其他的Generator是不合理的

INSTALL_COMMAND make install NO_SHARED=1 NO_LAPACK=1 PREFIX=<INSTALL_DIR>
UPDATE_COMMAND ""
CONFIGURE_COMMAND ""
Expand Down
3 changes: 2 additions & 1 deletion cmake/external/rocksdb.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ ExternalProject_Add(
${ROCKSDB_PREFIX_DIR}/src/extern_rocksdb/librocksdb.a ${ROCKSDB_LIBRARIES}
&& cp -r ${ROCKSDB_PREFIX_DIR}/src/extern_rocksdb/include
${ROCKSDB_INSTALL_DIR}/
BUILD_IN_SOURCE 1)
BUILD_IN_SOURCE 1
BYPRODUCTS ${ROCKSDB_LIBRARIES})

add_dependencies(extern_rocksdb snappy)

Expand Down
3 changes: 3 additions & 0 deletions cmake/third_party.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ set(THIRD_PARTY_CACHE_PATH
set(THIRD_PARTY_BUILD_TYPE Release)
set(third_party_deps)

include(ProcessorCount)
ProcessorCount(NPROC)

# cache funciton to avoid repeat download code of third_party.
# This function has 4 parameters, URL / REPOSITOR / TAG / DIR:
# 1. URL: specify download url of 3rd party
Expand Down