From 945672bf3e8991a292ff466c04c84558c0e24b39 Mon Sep 17 00:00:00 2001 From: Wojciech Baranowski Date: Wed, 6 May 2020 14:23:00 -0700 Subject: [PATCH] cmake: improve dependencies in incremental builds (#37661) Summary: Fixes https://github.com/pytorch/pytorch/issues/26304 Test procedure: With ninja: [x] Build a clean checkout [x] Build again. Result: Only 10 libraries are (needlessly) linked again, the extra delay on a 24-core machine is <10s. [x] Build for the third time. Result: Virtually instantaneous, with no extra rebuilding. [x] Modify DispatchTable.h. Build again. Result: `.cu` files are rebuilt, as well as many `.cpp` files [x] Build for the fifth time. Result: Virtually instantaneous, with no extra rebuilding. [x] Touch one of the `.depend` files. Build again. Result: Only 10 libraries are (needlessly) linked again, the extra delay on a 24-core machine is <10s. Without ninja: [x] Build a clean checkout [x] Build again. Result: There is some unnecessary rebuilding. But it was also happening before this change. [x] Build for the third time. Result: Virtually instantaneous, with no extra rebuilding. Pull Request resolved: https://github.com/pytorch/pytorch/pull/37661 Differential Revision: D21434624 Pulled By: ezyang fbshipit-source-id: 379d2315486b8bb5972c184f9b8da8e00d38c338 --- CMakeLists.txt | 4 ++++ cmake/Dependencies.cmake | 16 +++++++++++++--- tools/setup_helpers/cmake.py | 16 ---------------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e0a5434400f..e777c4b40eb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,6 +52,10 @@ set(CMAKE_EXPORT_COMPILE_COMMANDS ON) # endif() set(CAFFE2_CMAKE_BUILDING_WITH_MAIN_REPO ON) +# Googletest's cmake files are going to set it on once they are processed. Let's +# set it at the very beginning so that the entire build is deterministic. +set(THREADS_PREFER_PTHREAD_FLAG ON) + if(NOT DEFINED BLAS_SET_BY_USER) if(DEFINED BLAS) set(BLAS_SET_BY_USER TRUE) diff --git a/cmake/Dependencies.cmake b/cmake/Dependencies.cmake index 0a60e3267b5..cd313a4f37f 100644 --- a/cmake/Dependencies.cmake +++ b/cmake/Dependencies.cmake @@ -881,9 +881,11 @@ if(BUILD_PYTHON) endif() # ---[ pybind11 -find_package(pybind11 CONFIG) -if(NOT pybind11_FOUND) - find_package(pybind11) +if(NOT ${pybind11_PREFER_third_party}) + find_package(pybind11 CONFIG) + if(NOT pybind11_FOUND) + find_package(pybind11) + endif() endif() if(pybind11_FOUND) @@ -894,6 +896,9 @@ else() install(DIRECTORY ${pybind11_INCLUDE_DIRS} DESTINATION ${CMAKE_INSTALL_PREFIX} FILES_MATCHING PATTERN "*.h") + set(pybind11_PREFER_third_party ON CACHE BOOL + "Use the third_party/pybind11 submodule, instead of looking for system + installation of pybind11") endif() message(STATUS "pybind11 include dirs: " "${pybind11_INCLUDE_DIRS}") include_directories(SYSTEM ${pybind11_INCLUDE_DIRS}) @@ -1125,7 +1130,12 @@ if(USE_ROCM) else() caffe2_update_option(USE_ROCM OFF) endif() +endif() +# ---[ ROCm +if(USE_ROCM) + # We check again for USE_ROCM because it might have been set to OFF + # in the if above include_directories(SYSTEM ${HIP_PATH}/include) include_directories(SYSTEM ${ROCBLAS_PATH}/include) include_directories(SYSTEM ${ROCFFT_PATH}/include) diff --git a/tools/setup_helpers/cmake.py b/tools/setup_helpers/cmake.py index 9e7dfda1342..f7b4706a4c4 100644 --- a/tools/setup_helpers/cmake.py +++ b/tools/setup_helpers/cmake.py @@ -338,19 +338,3 @@ class CMake: else: build_args += ['--', '-j', max_jobs] self.run(build_args, my_env) - - # in cmake, .cu compilation involves generating certain intermediates - # such as .cu.o and .cu.depend, and these intermediates finally get compiled - # into the final .so. - # Ninja updates build.ninja's timestamp after all dependent files have been built, - # and re-kicks cmake on incremental builds if any of the dependent files - # have a timestamp newer than build.ninja's timestamp. - # There is a cmake bug with the Ninja backend, where the .cu.depend files - # are still compiling by the time the build.ninja timestamp is updated, - # so the .cu.depend file's newer timestamp is screwing with ninja's incremental - # build detector. - # This line works around that bug by manually updating the build.ninja timestamp - # after the entire build is finished. - ninja_build_file = os.path.join(self.build_dir, 'build.ninja') - if os.path.exists(ninja_build_file): - os.utime(ninja_build_file, None)