Caffe2 module update: move observers as well as binaries. (#2145)

* Caffe2 module update: move observers as well as binaries.

* Add threads linkage

* Add Threads dependency to public interface

CMakeLists.txt

@@ -32,7 +32,6 @@ option(BUILD_SHARED_LIBS "Build libcaffe2.so" ON)
 cmake_dependent_option(
     CAFFE2_USE_MSVC_STATIC_RUNTIME "Using MSVC static runtime libraries" ON
     "NOT BUILD_SHARED_LIBS" OFF)
-option(BUILD_OBSERVERS "Build performance observers/loggers in caffe2/share/observers directory" OFF)
 option(BUILD_TEST "Build C++ test binaries (need gtest and gbenchmark)" ON)
 option(USE_ACL "Use ARM Compute Library" OFF)
 option(USE_ASAN "Use Address Sanitizer" OFF)
@@ -54,7 +53,7 @@ option(USE_NERVANA_GPU "Use Nervana GPU backend" OFF)
 option(USE_NNAPI "Use NNAPI" OFF)
 option(USE_NNPACK "Use NNPACK" ON)
 option(USE_NUMA "Use NUMA (only available on Linux)" ON)
-option(USE_OBSERVERS "Use Observer Library" OFF)
+option(USE_OBSERVERS "Use observers module." OFF)
 option(USE_OPENCV "Use openCV" ON)
 option(USE_OPENMP "Use OpenMP for parallel code" OFF)
 option(USE_PROF "Use profiling" OFF)
@@ -254,6 +253,7 @@ if (BUILD_SHARED_LIBS)
       ${PROJECT_SOURCE_DIR}/cmake/public/glog.cmake
       ${PROJECT_SOURCE_DIR}/cmake/public/gflags.cmake
       ${PROJECT_SOURCE_DIR}/cmake/public/protobuf.cmake
+      ${PROJECT_SOURCE_DIR}/cmake/public/threads.cmake
       ${PROJECT_SOURCE_DIR}/cmake/public/utils.cmake
       DESTINATION share/cmake/Caffe2/public
       COMPONENT dev)
@@ -269,5 +269,15 @@ endif()
 # ---[ Modules
 add_subdirectory(modules)
 
+# ---[ Binaries
+# Binaries will be built after the Caffe2 main libraries and the modules
+# are built. For the binaries, they will be linked to the Caffe2 main
+# libraries, as well as all the modules that are built with Caffe2 (the ones
+# built in the previous Modules section above).
+
+if (BUILD_BINARY)
+  add_subdirectory(binaries)
+endif()
+
 include(cmake/Summary.cmake)
 caffe2_print_configuration_summary()

binaries/CMakeLists.txt

@@ -8,10 +8,7 @@ caffe2_binary_target("run_plan.cc")
 caffe2_binary_target("speed_benchmark.cc")
 caffe2_binary_target("split_db.cc")
 
-if (USE_THREADS)
-  caffe2_binary_target("db_throughput.cc")
-  target_link_libraries(db_throughput ${CMAKE_THREAD_LIBS_INIT})
-endif()
+caffe2_binary_target("db_throughput.cc")
 
 if (USE_CUDA)
   caffe2_binary_target("inspect_gpus.cc")
@@ -44,7 +41,11 @@ endif()
 
 if (USE_OPENCV)
   caffe2_binary_target("make_image_db.cc")
-  target_link_libraries(make_image_db ${OpenCV_LIBS} ${CMAKE_THREAD_LIBS_INIT})
+  target_link_libraries(make_image_db ${OpenCV_LIBS})
+endif()
+
+if (USE_OBSERVERS)
+  caffe2_binary_target("caffe2_benchmark.cc")
 endif()
 
 # ---[ tutorials

binaries/caffe2_benchmark.cc

@@ -7,10 +7,11 @@
 #include "caffe2/core/logging.h"
 #include "caffe2/core/operator.h"
 #include "caffe2/proto/caffe2.pb.h"
-#include "caffe2/share/contrib/observers/observer_config.h"
 #include "caffe2/utils/proto_utils.h"
 #include "caffe2/utils/string_utils.h"
 
+#include "observers/observer_config.h"
+
 CAFFE2_DEFINE_string(
     backend,
     "builtin",

caffe2/CMakeLists.txt

@@ -78,6 +78,7 @@ install(FILES ${PROJECT_BINARY_DIR}/caffe2/core/macros.h
 
 # Compile exposed libraries.
 add_library(caffe2 ${Caffe2_CPU_SRCS} $<TARGET_OBJECTS:Caffe_PROTO> $<TARGET_OBJECTS:Caffe2_PROTO>)
+add_dependencies(caffe2 Caffe_PROTO Caffe2_PROTO)
 target_link_libraries(caffe2 PUBLIC ${Caffe2_PUBLIC_DEPENDENCY_LIBS})
 target_link_libraries(caffe2 PRIVATE ${Caffe2_DEPENDENCY_LIBS})
 target_include_directories(caffe2 INTERFACE $<INSTALL_INTERFACE:include>)
@@ -262,15 +263,7 @@ if (BUILD_PYTHON)
           FILES_MATCHING PATTERN "*.py")
 endif()
 
+# Finally, set the Caffe2_MAIN_LIBS variable in the parent scope.
+set(Caffe2_MAIN_LIBS ${Caffe2_MAIN_LIBS} PARENT_SCOPE)
+
 
-# ---[ Any builds that should happen after the main targets should be added here.
-# Binaries
-if (BUILD_BINARY)
-  add_subdirectory(binaries)
-  # Benchmarking binaries require observers included in the build
-  # There is a linking issue that happens in some of the Windows builds.
-  # TODO(Yangqing): after the module redesign, enable this back.
-  if (BUILD_OBSERVERS AND NOT MSVC)
-    add_subdirectory(share/contrib/binaries)
-  endif()
-endif()

caffe2/share/contrib/CMakeLists.txt

@@ -1,19 +1,10 @@
-set(Caffe2_CPU_OBSERVER_SRCS)
-
 if (USE_NNPACK)
   add_subdirectory(nnpack)
 endif()
-if (BUILD_OBSERVERS)
-add_subdirectory(observers)
-endif()
 if (USE_ZSTD)
   add_subdirectory(zstd)
 endif()
 
-if (BUILD_OBSERVERS)
-  add_library(Caffe2_CPU_OBSERVER STATIC ${Caffe2_CPU_OBSERVER_SRCS})
-endif()
-
 # CPU source, test sources, binary sources
 set(Caffe2_CPU_SRCS ${Caffe2_CPU_SRCS} PARENT_SCOPE)
 set(Caffe2_CPU_TEST_SRCS ${Caffe2_CPU_TEST_SRCS} PARENT_SCOPE)

caffe2/share/contrib/binaries/CMakeLists.txt

@@ -1 +0,0 @@
-add_subdirectory(caffe2_benchmark)

caffe2/share/contrib/binaries/caffe2_benchmark/CMakeLists.txt

@@ -1,3 +0,0 @@
-caffe2_binary_target("caffe2_benchmark.cc")
-caffe2_interface_library(Caffe2_CPU_OBSERVER Caffe2_CPU_OBSERVER_LINK)
-target_link_libraries(caffe2_benchmark ${Caffe2_CPU_OBSERVER_LINK})

caffe2/share/contrib/observers/CMakeLists.txt

@@ -1,7 +0,0 @@
-set(Caffe2_CPU_OBSERVER_SRCS
-  "${CMAKE_CURRENT_SOURCE_DIR}/net_observer_reporter_print.cc"
-  "${CMAKE_CURRENT_SOURCE_DIR}/observer_config.cc"
-  "${CMAKE_CURRENT_SOURCE_DIR}/perf_observer.cc"
-  )
-
-set(Caffe2_CPU_OBSERVER_SRCS ${Caffe2_CPU_OBSERVER_SRCS} PARENT_SCOPE)

cmake/BuildVariables.cmake

@@ -28,4 +28,9 @@ set(Caffe2_CUDA_DEPENDENCY_LIBS "")
 # Lists for Caffe2 public dependency libraries. These libraries will be
 # transitive to any libraries that depends on Caffe2.
 set(Caffe2_PUBLIC_DEPENDENCY_LIBS "")
-set(Caffe2_PUBLIC_CUDA_DEPENDENCY_LIBS "")
+set(Caffe2_PUBLIC_CUDA_DEPENDENCY_LIBS "")
+
+# List of modules that is built as part of the main Caffe2 build. For all
+# binary targets, such as Python and native binaries, they will be linked
+# automatically with these modules.
+set(Caffe2_MODULES "")

cmake/Caffe2Config.cmake.in

@@ -13,6 +13,11 @@ set(CAFFE2_VERSION "@CAFFE2_VERSION@")
 # Utils functions.
 include("${CMAKE_CURRENT_LIST_DIR}/public/utils.cmake")
 
+# Depending on whether Caffe2 is built with threads, include threads lib.
+if(@USE_THREADS@)
+  include("${CMAKE_CURRENT_LIST_DIR}/public/threads.cmake")
+endif()
+
 # Depending on whether Caffe2 uses gflags during compile time or
 # not, invoke gflags.
 if (@USE_GFLAGS@)

cmake/Dependencies.cmake

@@ -3,8 +3,8 @@ include("cmake/ProtoBuf.cmake")
 
 # ---[ Threads
 if(USE_THREADS)
-  find_package(Threads REQUIRED)
-  list(APPEND Caffe2_DEPENDENCY_LIBS ${CMAKE_THREAD_LIBS_INIT})
+  include(cmake/public/threads.cmake)
+  list(APPEND Caffe2_PUBLIC_DEPENDENCY_LIBS Threads::Threads)
 endif()
 
 # ---[ protobuf
@@ -77,10 +77,6 @@ if(USE_NNPACK)
   endif()
 endif()
 
-if(USE_OBSERVERS)
-  list(APPEND Caffe2_DEPENDENCY_LIBS Caffe2_CPU_OBSERVER)
-endif()
-
 # ---[ On Android, Caffe2 uses cpufeatures library in the thread pool
 if (ANDROID)
   if (NOT TARGET cpufeatures)

cmake/public/glog.cmake

@@ -41,7 +41,7 @@ else()
       find_path(GLOG_INCLUDE_DIR glog/logging.h
           PATHS ${GLOG_ROOT_DIR})
   endif()
-  
+
   find_library(GLOG_LIBRARY glog
       PATHS ${GLOG_ROOT_DIR}
       PATH_SUFFIXES lib lib64)

cmake/public/protobuf.cmake

@@ -64,7 +64,7 @@ elseif(Protobuf_FOUND OR PROTOBUF_FOUND)
           IMPORTED_LOCATION_DEBUG "${PROTOBUF_LITE_LIBRARY_DEBUG}")
     endif()
   endif()
-  
+
   if(PROTOBUF_PROTOC_EXECUTABLE)
     if (NOT TARGET protobuf::protoc)
       add_executable(protobuf::protoc IMPORTED)

cmake/public/threads.cmake

@@ -0,0 +1,18 @@
+find_package(Threads REQUIRED)
+# For newer CMake, Threads::Threads is already defined. Otherwise, we will
+# provide a backward compatible wrapper for Threads::Threads.
+if(THREADS_FOUND AND NOT TARGET Threads::Threads)
+  add_library(Threads::Threads INTERFACE IMPORTED)
+
+  if(THREADS_HAVE_PTHREAD_ARG)
+    set_property(
+        TARGET Threads::Threads
+        PROPERTY INTERFACE_COMPILE_OPTIONS "-pthread")
+  endif()
+
+  if(CMAKE_THREAD_LIBS_INIT)
+    set_property(
+        TARGET Threads::Threads
+        PROPERTY INTERFACE_LINK_LIBRARIES "${CMAKE_THREAD_LIBS_INIT}")
+  endif()
+endif()

cmake/public/utils.cmake

@@ -90,5 +90,9 @@ function(caffe2_binary_target target_name_or_src)
   endif()
   add_executable(${__target} ${__srcs})
   target_link_libraries(${__target} ${Caffe2_MAIN_LIBS})
+  # If we have Caffe2_MODULES defined, we will also link with the modules.
+  if (DEFINED Caffe2_MODULES)
+    target_link_libraries(${__target} ${Caffe2_MODULES})
+  endif()
   install(TARGETS ${__target} DESTINATION bin)
 endfunction()

modules/CMakeLists.txt

@@ -1,4 +1,8 @@
 # ---[ Add modules
 add_subdirectory(detectron)
 add_subdirectory(module_test)
+add_subdirectory(observers)
 add_subdirectory(rocksdb)
+
+# Finally, set Caffe2_MODULES to parent scope.
+set(Caffe2_MODULES ${Caffe2_MODULES} PARENT_SCOPE)

modules/module_test/module_test_dynamic.cc

@@ -19,7 +19,7 @@
 
 // An explicitly defined module, testing correctness when we dynamically link a
 // module
-CAFFE2_MODULE(caffe2_module_test_dynamic, "Dynamic module for testing.");
+CAFFE2_MODULE(caffe2_module_test_dynamic, "Dynamic module only used for testing.");
 
 namespace caffe2 {
 

modules/observers/CMakeLists.txt

@@ -0,0 +1,25 @@
+if (CAFFE2_CMAKE_BUILDING_WITH_MAIN_REPO)
+  if (NOT USE_OBSERVERS)
+    return()
+  endif()
+else()
+  cmake_minimum_required(VERSION 3.0 FATAL_ERROR)
+  project(caffe2_observers CXX)
+  find_package(Caffe2 REQUIRED)
+  option(BUILD_SHARED_LIBS "Build shared libs." ON)
+endif()
+
+add_library(caffe2_observers
+    "${CMAKE_CURRENT_SOURCE_DIR}/net_observer_reporter_print.cc"
+    "${CMAKE_CURRENT_SOURCE_DIR}/observer_config.cc"
+    "${CMAKE_CURRENT_SOURCE_DIR}/perf_observer.cc"
+    )
+target_link_libraries(caffe2_observers PUBLIC caffe2_library)
+target_include_directories(caffe2_observers PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/..)
+install(TARGETS caffe2_observers DESTINATION lib)
+caffe2_interface_library(caffe2_observers caffe2_observers_library)
+
+if (CAFFE2_CMAKE_BUILDING_WITH_MAIN_REPO)
+  set(Caffe2_MODULES ${Caffe2_MODULES} caffe2_observers_library PARENT_SCOPE)
+endif()
+

modules/observers/net_observer_reporter_print.cc

@@ -1,7 +1,7 @@
-#include "caffe2/share/contrib/observers/net_observer_reporter_print.h"
+#include "observers/net_observer_reporter_print.h"
 
 #include "caffe2/core/init.h"
-#include "caffe2/share/contrib/observers/observer_config.h"
+#include "observers/observer_config.h"
 
 namespace caffe2 {
 

modules/observers/net_observer_reporter_print.h

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "caffe2/share/contrib/observers/net_observer_reporter.h"
+#include "observers/net_observer_reporter.h"
 
 namespace caffe2 {
 

modules/observers/observer_config.cc

@@ -1,4 +1,4 @@
-#include "caffe2/share/contrib/observers/observer_config.h"
+#include "observers/observer_config.h"
 
 namespace caffe2 {
 

modules/observers/observer_config.h

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "caffe2/share/contrib/observers/net_observer_reporter.h"
+#include "observers/net_observer_reporter.h"
 
 namespace caffe2 {
 

modules/observers/perf_observer.cc

@@ -1,5 +1,5 @@
-#include "caffe2/share/contrib/observers/perf_observer.h"
-#include "caffe2/share/contrib/observers/observer_config.h"
+#include "observers/perf_observer.h"
+#include "observers/observer_config.h"
 
 #include <random>
 #include "caffe2/core/common.h"

modules/rocksdb/CMakeLists.txt

@@ -64,5 +64,5 @@ install(TARGETS caffe2_rocksdb DESTINATION lib)
 # Note(jiayq): this also depends on a separate cmake move to reorg test builds
 # and binary builds after modules. When it is done, this note should be removed.
 if (CAFFE2_CMAKE_BUILDING_WITH_MAIN_REPO)
-  list(APPEND Caffe2_MAIN_LIBS caffe2_rocksdb)
+  set(Caffe2_MODULES ${Caffe2_MODULES} caffe2_rocksdb PARENT_SCOPE)
 endif()

modules/rocksdb/rocksdb.cc

@@ -110,4 +110,6 @@ REGISTER_CAFFE2_DB(RocksDB, RocksDB);
 REGISTER_CAFFE2_DB(rocksdb, RocksDB);
 
 }  // namespace db
+
+CAFFE2_MODULE(caffe2_rocksdb, "RocksDB implementation for caffe2::DB.");
 }  // namespace caffe2