Merge pull request ceph#16755 from ivancich/wip-pull-new-dmclock

osd: bring in dmclock library changes Reviewed-by: J. Eric Ivancich <ivancich@redhat.com> Reviewed-by: Greg Farnum <gfarnum@redhat.com>
ivancich · Aug 8, 2017 · 25f1ede · 25f1ede
2 parents 6127a4c + 8e92496
commit 25f1ede
Show file tree

Hide file tree

Showing 16 changed files with 198 additions and 79 deletions.
diff --git a/src/common/mClockPriorityQueue.h b/src/common/mClockPriorityQueue.h
@@ -298,7 +298,7 @@ namespace ceph {
 
     void enqueue(K cl, unsigned priority, unsigned cost, T item) override final {
       // priority is ignored
-      queue.add_request(item, cl, cost);
+      queue.add_request(std::move(item), cl, cost);
     }
 
     void enqueue_front(K cl,

diff --git a/src/dmclock/CMakeLists.txt b/src/dmclock/CMakeLists.txt
@@ -2,17 +2,26 @@ cmake_minimum_required(VERSION 2.8.11)
 
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake/modules")
 
+set(CMAKE_CXX_FLAGS
+  "${CMAKE_CXX_FLAGS} -std=c++11 -Wno-write-strings -Wall -pthread")
+
+if(DO_NOT_DELAY_TAG_CALC)
+  add_definitions(-DDO_NOT_DELAY_TAG_CALC)
+endif()
+
 if (NOT(TARGET gtest AND TARGET gtest_main))
-  if(NOT(GTEST_FOUND))
-    find_package(GTest REQUIRED)
+  if (NOT GTEST_FOUND)
+    find_package(GTest QUIET)
+    if (NOT GTEST_FOUND)
+      include(BuildGTest)
+    endif()
   endif()
 endif()
 
 if (NOT(BOOST_FOUND))
   find_package(Boost REQUIRED)
 endif()
 
-# add_subdirectory(support/src)
 add_subdirectory(src)
 add_subdirectory(sim)
 

diff --git a/src/dmclock/cmake/modules/BuildGTest.cmake b/src/dmclock/cmake/modules/BuildGTest.cmake
@@ -0,0 +1,67 @@
+macro(_build_gtest gtest_root)
+  include(ExternalProject)
+  ExternalProject_Add(googletest
+    SOURCE_DIR ${gtest_root}
+    CMAKE_ARGS -DBUILD_GMOCK=OFF -DBUILD_GTEST=ON
+    INSTALL_COMMAND ""
+    LOG_CONFIGURE ON
+    LOG_BUILD ON)
+
+  ExternalProject_Get_Property(googletest source_dir)
+  set(GTEST_INCLUDE_DIRS ${source_dir}/googletest/include)
+  set(GMOCK_INCLUDE_DIRS ${source_dir}/googlemock/include)
+
+  find_package(Threads REQUIRED)
+
+  ExternalProject_Get_Property(googletest binary_dir)
+  set(GTEST_LIBRARY_PATH ${binary_dir}/googletest/${CMAKE_FIND_LIBRARY_PREFIXES}gtest.a)
+  set(GTEST_LIBRARY gtest)
+  add_library(${GTEST_LIBRARY} STATIC IMPORTED)
+  set_target_properties(${GTEST_LIBRARY} PROPERTIES
+    INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIRS}"
+    IMPORTED_LOCATION ${GTEST_LIBRARY_PATH}
+    IMPORTED_LINK_INTERFACE_LANGUAGES "CXX"
+    IMPORTED_LINK_INTERFACE_LIBRARIES ${CMAKE_THREAD_LIBS_INIT})
+  add_dependencies(${GTEST_LIBRARY} googletest)
+  set(GTEST_LIBRARIES ${GTEST_LIBRARY})
+
+  set(GTEST_MAIN_LIBRARY_PATH ${binary_dir}/googletest/${CMAKE_FIND_LIBRARY_PREFIXES}gtest_main.a)
+  set(GTEST_MAIN_LIBRARY gtest_main)
+  add_library(${GTEST_MAIN_LIBRARY} STATIC IMPORTED)
+  set_target_properties(${GTEST_MAIN_LIBRARY} PROPERTIES
+    INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIRS}"
+    IMPORTED_LOCATION ${GTEST_MAIN_LIBRARY_PATH}
+    IMPORTED_LINK_INTERFACE_LIBRARIES ${CMAKE_THREAD_LIBS_INIT})
+  add_dependencies(${GTEST_MAIN_LIBRARY} googletest)
+
+  set(GMOCK_LIBRARY_PATH ${binary_dir}/googlemock/${CMAKE_FIND_LIBRARY_PREFIXES}gmock.a)
+  set(GMOCK_LIBRARY gmock)
+  add_library(${GMOCK_LIBRARY} STATIC IMPORTED)
+  set_target_properties(${GMOCK_LIBRARY} PROPERTIES
+    INTERFACE_INCLUDE_DIRECTORIES "${GMOCK_INCLUDE_DIRS}"
+    IMPORTED_LOCATION "${GMOCK_LIBRARY_PATH}"
+    IMPORTED_LINK_INTERFACE_LANGUAGES "CXX"
+    IMPORTED_LINK_INTERFACE_LIBRARIES ${CMAKE_THREAD_LIBS_INIT})
+  add_dependencies(${GMOCK_LIBRARY} googletest)
+
+  set(GMOCK_MAIN_LIBRARY_PATH ${binary_dir}/googlemock/${CMAKE_FIND_LIBRARY_PREFIXES}gmock_main.a)
+  set(GMOCK_MAIN_LIBRARY gmock_main)
+  add_library(${GMOCK_MAIN_LIBRARY} STATIC IMPORTED)
+  set_target_properties(${GMOCK_MAIN_LIBRARY} PROPERTIES
+    INTERFACE_INCLUDE_DIRECTORIES "${GMOCK_INCLUDE_DIRS}"
+    IMPORTED_LOCATION ${GMOCK_MAIN_LIBRARY_PATH}
+    IMPORTED_LINK_INTERFACE_LANGUAGES "CXX"
+    IMPORTED_LINK_INTERFACE_LIBRARIES ${CMAKE_THREAD_LIBS_INIT})
+  add_dependencies(${GMOCK_MAIN_LIBRARY} ${GTEST_LIBRARY})
+endmacro()
+
+find_path(GTEST_ROOT src/gtest.cc
+  HINTS $ENV{GTEST_ROOT}
+  PATHS /usr/src/googletest/googletest /usr/src/gtest)
+
+if(EXISTS ${GTEST_ROOT})
+  message(STATUS "Found googletest: ${GTEST_ROOT}")
+  _build_gtest(${GTEST_ROOT})
+else()
+  message(SEND_ERROR "Could NOT find googletest")
+endif()
diff --git a/src/dmclock/sim/src/sim_client.h b/src/dmclock/sim/src/sim_client.h
@@ -89,7 +89,7 @@ namespace crimson {
 
       using SubmitFunc =
 	std::function<void(const ServerId&,
-			   const TestRequest&,
+			   TestRequest&&,
 			   const ClientId&,
 			   const ReqPm&)>;
 
@@ -240,8 +240,9 @@ namespace crimson {
 	      count_stats(internal_stats.mtx,
 			  internal_stats.get_req_params_count);
 
-	      TestRequest req(server, o, 12);
-	      submit_f(server, req, id, rp);
+	      submit_f(server,
+		       TestRequest{server, static_cast<uint32_t>(o), 12},
+		       id, rp);
 	      ++outstanding_ops;
 	      l.lock(); // lock for return to top of loop
 

diff --git a/src/dmclock/sim/src/sim_server.h b/src/dmclock/sim/src/sim_server.h
@@ -147,14 +147,15 @@ namespace crimson {
 	delete priority_queue;
       }
 
-      void post(const TestRequest& request,
+      void post(TestRequest&& request,
 		const ClientId& client_id,
 		const ReqPm& req_params)
       {
 	time_stats(internal_stats.mtx,
 		   internal_stats.add_request_time,
 		   [&](){
-		     priority_queue->add_request(request, client_id, req_params);
+		     priority_queue->add_request(std::move(request),
+						 client_id, req_params);
 		   });
 	count_stats(internal_stats.mtx,
 		    internal_stats.add_request_count);
@@ -202,10 +203,9 @@ namespace crimson {
 	    // notify server of completion
 	    std::this_thread::sleep_for(op_time);
 
-	    TestResponse resp(req->epoch);
 	    // TODO: rather than assuming this constructor exists, perhaps
 	    // pass in a function that does this mapping?
-	    client_resp_f(client, resp, id, additional);
+	    client_resp_f(client, TestResponse{req->epoch}, id, additional);
 
 	    time_stats(internal_stats.mtx,
 		       internal_stats.request_complete_time,

diff --git a/src/dmclock/sim/src/ssched/ssched_server.h b/src/dmclock/sim/src/ssched/ssched_server.h
@@ -100,10 +100,11 @@ namespace crimson {
 	finishing = true;
       }
 
-      void add_request(const R& request,
+      void add_request(R&& request,
 		       const C& client_id,
 		       const ReqParams& req_params) {
-	add_request(RequestRef(new R(request)), client_id, req_params);
+	add_request(RequestRef(new R(std::move(request))),
+		    client_id, req_params);
       }
 
       void add_request(RequestRef&& request,

diff --git a/src/dmclock/sim/src/test_dmclock_main.cc b/src/dmclock/sim/src/test_dmclock_main.cc
@@ -136,11 +136,11 @@ int main(int argc, char* argv[]) {
     // lambda to post a request to the identified server; called by client
     test::SubmitFunc server_post_f =
         [&simulation](const ServerId& server,
-                      const sim::TestRequest& request,
+                      sim::TestRequest&& request,
                       const ClientId& client_id,
                       const test::dmc::ReqParams& req_params) {
         test::DmcServer& s = simulation->get_server(server);
-        s.post(request, client_id, req_params);
+        s.post(std::move(request), client_id, req_params);
     };
 
     std::vector<std::vector<sim::CliInst>> cli_inst;

diff --git a/src/dmclock/sim/src/test_ssched_main.cc b/src/dmclock/sim/src/test_ssched_main.cc
@@ -67,11 +67,11 @@ int main(int argc, char* argv[]) {
   // lambda to post a request to the identified server; called by client
   test::SubmitFunc server_post_f =
     [&simulation](const ServerId& server_id,
-		  const sim::TestRequest& request,
+		  sim::TestRequest&& request,
 		  const ClientId& client_id,
 		  const ssched::ReqParams& req_params) {
     auto& server = simulation->get_server(server_id);
-    server.post(request, client_id, req_params);
+    server.post(std::move(request), client_id, req_params);
   };
 
   static std::vector<sim::CliInst> no_wait =

diff --git a/src/dmclock/src/CMakeLists.txt b/src/dmclock/src/CMakeLists.txt
@@ -2,23 +2,9 @@ include_directories(${GTEST_INCLUDE_DIRS})
 include_directories(${Boost_INCLUDE_DIRS})
 include_directories(../support/src)
 
-set(local_flags "-std=c++11 -Wno-write-strings -Wall -pthread")
-
-if(DO_NOT_DELAY_TAG_CALC)
-  set(local_flags "${local_flags} -DDO_NOT_DELAY_TAG_CALC")
-endif()
+set(CMAKE_CXX_FLAGS
+  "${CMAKE_CXX_FLAGS} -std=c++11 -Wno-write-strings -Wall -pthread")
 
 set(dmc_srcs dmclock_util.cc ../support/src/run_every.cc)
 
-set_source_files_properties(${dmc_srcs}
-  PROPERTIES
-  COMPILE_FLAGS "${local_flags}"
-  )
-
-if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
-  set(warnings_off " -Wno-unused-variable -Wno-unused-function")
-elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
-  set(warnings_off " -Wno-unused-but-set-variable -Wno-unused-function")
-endif()
-
 add_library(dmclock STATIC ${dmc_srcs})
diff --git a/src/dmclock/src/dmclock_client.h b/src/dmclock/src/dmclock_client.h
@@ -19,8 +19,6 @@
 #include "dmclock_util.h"
 #include "dmclock_recs.h"
 
-#include "gtest/gtest_prod.h"
-
 
 namespace crimson {
   namespace dmclock {
@@ -57,7 +55,8 @@ namespace crimson {
     // S is server identifier type
     template<typename S>
     class ServiceTracker {
-      FRIEND_TEST(dmclock_client, server_erase);
+      // we don't want to include gtest.h just for FRIEND_TEST
+      friend class dmclock_client_server_erase_Test;
 
       using TimePoint = decltype(std::chrono::steady_clock::now());
       using Duration = std::chrono::milliseconds;