Squashed 'src/dmclock/' changes from d6586d7..64bcc6e

64bcc6e Remove FindGTest and FindBoost, which already exist in cmake required version 2.8.11 and later. Also clean up.
ed6dc60 Do some clean-up and remove memory leaks from tests and simulations.
360172a Allow RunEvery object to be ended before destruction via "join" member function.
135b5d0 Fix two bugs discovered by valgrind. Do some other code clean-up as well.
a26d29e Modify cmake files to match naming of cmake's upstream gtest and ceph's boost.
e23e63f Reorganize CMakeLists.txt files, so that projects that incorporate dmclock can control whether add_test is called. Now, add_test is called only at the top-level. Projects that incorporate dmclock can use add_subdirectory on subdirectories beneath the top-level to selective exactly what is included.

git-subtree-dir: src/dmclock
git-subtree-split: 64bcc6ea697c7e790ec4a4229d1bb1d492f9e803

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 428863dc496df44f3adb733b75e5af564a25dd43..9444ea1e96afd0a27250d6d989a0b41f797cdcd2 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,32 +1,25 @@
 cmake_minimum_required(VERSION 2.8.11)
 
-set(CMAKE_CXX_FLAGS "-std=c++11 -Wno-write-strings ${CMAKE_CXX_FLAGS}")
+list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake/modules")
 
-set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake/modules/")
-
-if(DO_NOT_DELAY_TAG_CALC)
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DDO_NOT_DELAY_TAG_CALC")
-endif()
-
-if(K_WAY_HEAP)
-  if(K_WAY_HEAP LESS 2)
-    message(FATAL_ERROR "K_WAY_HEAP value should be at least 2")
-  else()
-    set(CMAKE_CXX_SIM_FLAGS "-DK_WAY_HEAP=${K_WAY_HEAP}")
+if (NOT(TARGET gtest AND TARGET gtest_main))
+  if(NOT(GTEST_FOUND))
+    find_package(GTest REQUIRED)
   endif()
 endif()
 
-if (NOT(TARGET gtest AND TARGET gtest_main))
-  find_package(gtest REQUIRED)
-  include_directories(${GTEST_INCLUDE_DIRS})
+if (NOT(BOOST_FOUND))
+  find_package(Boost REQUIRED)
 endif()
 
-find_package(Boost REQUIRED)
-include_directories(${Boost_INCLUDE_DIRS})
-
+# add_subdirectory(support/src)
 add_subdirectory(src)
 add_subdirectory(sim)
-add_subdirectory(support)
 
 enable_testing()
 add_subdirectory(test)
+add_subdirectory(support/test)
+add_test(NAME dmclock-tests
+  COMMAND $<TARGET_FILE:dmclock-tests>)
+add_test(NAME dmclock-data-struct-tests
+  COMMAND $<TARGET_FILE:dmclock-data-struct-tests>)

diff --git a/cmake/modules/Findboost.cmake b/cmake/modules/Findboost.cmake
deleted file mode 100644 (file)
index 4f0dfd0..0000000
--- a/cmake/modules/Findboost.cmake
+++ /dev/null
@@ -1,15 +0,0 @@
-# - Find boost
-
-find_path(BOOST_INCLUDE_DIR NAMES boost/variant.hpp
-    PATHS /usr/include /usr/local/include ${BOOST_DIR}/include)
-
-include(FindPackageHandleStandardArgs)
-FIND_PACKAGE_HANDLE_STANDARD_ARGS(boost
-  REQUIRED_VARS BOOST_INCLUDE_DIR)
-
-if(boost_FOUND)
-  set(BOOST_FOUND 1)
-endif()
-if(BOOST_FOUND)
-  set(BOOST_INCLUDES ${BOOST_INCLUDE_DIR})
-endif()

diff --git a/cmake/modules/Findgtest.cmake b/cmake/modules/Findgtest.cmake
deleted file mode 100644 (file)
index bfe0980..0000000
--- a/cmake/modules/Findgtest.cmake
+++ /dev/null
@@ -1,48 +0,0 @@
-# - Find gtest
-#
-#  GTEST_INCLUDE_DIRS   - where to find mcas/mcas.h, etc.
-#  GTEST_LIBRARIES      - List of libraries when using mcas.
-#  GTEST_FOUND          - True if mcas found.
-#
-#  GMOCK_INCLUDE_DIRS   - where to find mcas/mcas.h, etc.
-#  GMOCK_LIBRARIES      - List of libraries when using mcas.
-#  GMOCK_FOUND          - True if mcas found.
-
-
-## GTEST
-
-find_path(GTEST_INCLUDE_DIRS NAMES gtest/gtest.h
-    PATHS /usr/include /usr/local/include)
-
-find_library(GTEST_LIBRARY gtest
-  PATHS /usr/local/lib /usr/lib64)
-
-find_library(GTEST_MAIN_LIBRARY gtest_main
-  PATHS /usr/local/lib /usr/lib64)
-
-include(FindPackageHandleStandardArgs)
-FIND_PACKAGE_HANDLE_STANDARD_ARGS(gtest
-  REQUIRED_VARS GTEST_LIBRARY GTEST_MAIN_LIBRARY GTEST_INCLUDE_DIRS)
-
-if(gtest_FOUND)
-  set(GTEST_FOUND 1)
-endif()
-
-## GMOCK
-
-find_path(GMOCK_INCLUDE_DIRS NAMES gmock/gmock.h
-    PATHS /usr/include /usr/local/include)
-
-find_library(GMOCK_LIBRARY gmock
-  PATHS /usr/local/lib /usr/lib64)
-
-find_library(GMOCK_MAIN_LIBRARY gmock_main
-  PATHS /usr/local/lib /usr/lib64)
-
-include(FindPackageHandleStandardArgs)
-FIND_PACKAGE_HANDLE_STANDARD_ARGS(gmock
-  REQUIRED_VARS GMOCK_LIBRARY GMOCK_MAIN_LIBRARY GMOCK_INCLUDE_DIRS)
-
-if(gmock_FOUND)
-  set(GMOCK_FOUND 1)
-endif()

diff --git a/sim/CMakeLists.txt b/sim/CMakeLists.txt
index febd4f0ab6f826fc669a9047b2c86fd7dc8c351d..c088d2f1cd7c96205a7183354f84e86df65b04be 100644 (file)
--- a/sim/CMakeLists.txt
+++ b/sim/CMakeLists.txt
@@ -1 +1,9 @@
+if(K_WAY_HEAP)
+  if(K_WAY_HEAP LESS 2)
+    message(FATAL_ERROR "K_WAY_HEAP value should be at least 2")
+  else()
+    set(CMAKE_CXX_SIM_FLAGS "-DK_WAY_HEAP=${K_WAY_HEAP}")
+  endif()
+endif()
+
 add_subdirectory(src)

diff --git a/sim/src/sim_server.h b/sim/src/sim_server.h
index a61cc3204e4e96dd5fae5c8f5051ff7e0540e0fd..e318d6e90ac328e6810aa4b7a1fc4f628003ee37 100644 (file)
--- a/sim/src/sim_server.h
+++ b/sim/src/sim_server.h
@@ -143,6 +143,8 @@ namespace crimson {
        }
 
        delete[] threads;
+
+       delete priority_queue;
       }
 
       void post(const TestRequest& request,

diff --git a/sim/src/simulate.h b/sim/src/simulate.h
index 18e752d8a354f513dcfeb58a34e8f70ee07e9ff5..a967d004b6dcfcb0658edc2aa7740a6e8ffb5a21 100644 (file)
--- a/sim/src/simulate.h
+++ b/sim/src/simulate.h
@@ -88,6 +88,17 @@ namespace crimson {
        // empty
       }
 
+      ~Simulation() {
+       for (auto c : clients) {
+         TC* cp = c.second;
+         delete cp;
+       }
+
+       for (auto s : servers) {
+         delete s.second;
+       }
+      }
+
       uint get_client_count() const { return client_count; }
       uint get_server_count() const { return server_count; }
       TC& get_client(ClientId id) { return *clients[id]; }

diff --git a/sim/src/test_dmclock_main.cc b/sim/src/test_dmclock_main.cc
index c3ba1e18fbd6d1a23c155ea26e1de6602aa2b3dd..668869b8d72fb040d3b74997db89a071e0f9a72a 100644 (file)
--- a/sim/src/test_dmclock_main.cc
+++ b/sim/src/test_dmclock_main.cc
@@ -226,6 +226,8 @@ int main(int argc, char* argv[]) {
     simulation->display_stats(std::cout,
                               &test::server_data, &test::client_data,
                               server_disp_filter, client_disp_filter);
+
+    delete simulation;
 } // main
 
 

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 691e64cce4389f914047ad49e45d49519984f2eb..7178f266e2a41d13276ad1fbe3a92bc49135cd37 100644 (file)
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -1,7 +1,12 @@
+include_directories(${GTEST_INCLUDE_DIRS})
+include_directories(${Boost_INCLUDE_DIRS})
 include_directories(../support/src)
-include_directories(${BOOST_INCLUDE_DIR})
 
-set(local_flags "-Wall -pthread")
+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(dmc_srcs dmclock_util.cc ../support/src/run_every.cc)
 

diff --git a/src/dmclock_server.h b/src/dmclock_server.h
index 65013063fa7156ea0384bceb8258fe5bab45c44c..8aaad3977264847c0c3351d76eff05da7d629171 100644 (file)
--- a/src/dmclock_server.h
+++ b/src/dmclock_server.h
@@ -117,23 +117,24 @@ namespace crimson {
 
       RequestTag(const RequestTag& prev_tag,
                 const ClientInfo& client,
-                const ReqParams& req_params,
-                const Time& time,
+                const uint32_t delta,
+                const uint32_t rho,
+                const Time time,
                 const double cost = 0.0) :
        reservation(cost + tag_calc(time,
                                    prev_tag.reservation,
                                    client.reservation_inv,
-                                   req_params.rho,
+                                   rho,
                                    true)),
        proportion(tag_calc(time,
                            prev_tag.proportion,
                            client.weight_inv,
-                           req_params.delta,
+                           delta,
                            true)),
        limit(tag_calc(time,
                       prev_tag.limit,
                       client.limit_inv,
-                      req_params.delta,
+                      delta,
                       false)),
        ready(false)
 #ifndef DO_NOT_DELAY_TAG_CALC
@@ -143,7 +144,15 @@ namespace crimson {
        assert(reservation < max_tag || proportion < max_tag);
       }
 
-      RequestTag(double _res, double _prop, double _lim, const Time& _arrival) :
+      RequestTag(const RequestTag& prev_tag,
+                const ClientInfo& client,
+                const ReqParams req_params,
+                const Time time,
+                const double cost = 0.0) :
+       RequestTag(prev_tag, client, req_params.delta, req_params.rho, time, cost)
+      { /* empty */ }
+
+      RequestTag(double _res, double _prop, double _lim, const Time _arrival) :
        reservation(_res),
        proportion(_prop),
        limit(_lim),
@@ -189,7 +198,7 @@ namespace crimson {
 
     private:
 
-      static double tag_calc(const Time& time,
+      static double tag_calc(const Time time,
                             double prev,
                             double increment,
                             uint32_t dist_req_val,
@@ -737,11 +746,11 @@ namespace crimson {
 
 
       // data_mtx must be held by caller
-      void do_add_request(RequestRef&&     request,
-                         const C&         client_id,
+      void do_add_request(RequestRef&& request,
+                         const C& client_id,
                          const ReqParams& req_params,
-                         const Time       time,
-                         const double     cost = 0.0) {
+                         const Time time,
+                         const double cost = 0.0) {
        ++tick;
 
        // this pointer will help us create a reference to a shared
@@ -822,8 +831,11 @@ namespace crimson {
        RequestTag tag(0, 0, 0, time);
 
        if (!client.has_request()) {
-         tag = RequestTag(client.get_req_tag(), client.info,
-                          req_params, time, cost);
+         tag = RequestTag(client.get_req_tag(),
+                          client.info,
+                          req_params,
+                          time,
+                          cost);
 
          // copy tag to previous tag for client
          client.update_req_tag(tag, tick);
@@ -867,8 +879,9 @@ namespace crimson {
                                                  RequestRef& request)> process) {
        // gain access to data
        ClientRec& top = heap.top();
-       ClientReq& first = top.next_request();
-       RequestRef request = std::move(first.request);
+
+       RequestRef request = std::move(top.next_request().request);
+       RequestTag tag = top.next_request().tag;
 
        // pop request and adjust heaps
        top.pop_request();
@@ -876,8 +889,8 @@ namespace crimson {
 #ifndef DO_NOT_DELAY_TAG_CALC
        if (top.has_request()) {
          ClientReq& next_first = top.next_request();
-         next_first.tag = RequestTag(first.tag, top.info,
-                                     ReqParams(top.cur_delta, top.cur_rho),
+         next_first.tag = RequestTag(tag, top.info,
+                                     top.cur_delta, top.cur_rho,
                                      next_first.tag.arrival);
 
          // copy tag to previous tag for client
@@ -1423,10 +1436,10 @@ namespace crimson {
 
 
       void add_request(typename super::RequestRef&& request,
-                      const C&         client_id,
+                      const C& client_id,
                       const ReqParams& req_params,
-                      const Time       time,
-                      double           addl_cost = 0.0) {
+                      const Time time,
+                      double addl_cost = 0.0) {
        typename super::DataGuard g(this->data_mtx);
 #ifdef PROFILE
        add_request_timer.start();
@@ -1577,6 +1590,7 @@ namespace crimson {
 
       void sched_at(Time when) {
        std::lock_guard<std::mutex> l(sched_ahead_mtx);
+       if (this->finishing) return;
        if (TimeZero == sched_ahead_when || when < sched_ahead_when) {
          sched_ahead_when = when;
          sched_ahead_cv.notify_one();

diff --git a/support/CMakeLists.txt b/support/CMakeLists.txt
deleted file mode 100644 (file)
index 552439e..0000000
--- a/support/CMakeLists.txt
+++ /dev/null
@@ -1 +0,0 @@
-add_subdirectory(test)

diff --git a/support/src/indirect_intrusive_heap.h b/support/src/indirect_intrusive_heap.h
index b6075bda22f92be2f5080b6a78fd0f7f4c741b74..6342e29b16370dc1a5508a360b9fa1c31581428c 100644 (file)
--- a/support/src/indirect_intrusive_heap.h
+++ b/support/src/indirect_intrusive_heap.h
@@ -243,7 +243,7 @@ namespace crimson {
     }
 
     void pop() {
-      remove(0);
+      remove(HeapIndex(0));
     }
 
     void remove(Iterator& i) {

diff --git a/support/src/run_every.cc b/support/src/run_every.cc
index 258baaa74c041bf3b6d0159b85c1135a82009fa5..cab414109a9dc6fcc648fff7873d432961d29cdb 100644 (file)
--- a/support/src/run_every.cc
+++ b/support/src/run_every.cc
@@ -1,12 +1,10 @@
 // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
 /*
- * Copyright (C) 2016 Red Hat Inc.
+ * Copyright (C) 2017 Red Hat Inc.
  */
 
 
-#include <iostream>
-
 #include "run_every.h"
 
 
@@ -53,8 +51,17 @@ crimson::RunEvery& crimson::RunEvery::operator=(crimson::RunEvery&& other)
 
 
 crimson::RunEvery::~RunEvery() {
-  finishing = true;
-  cv.notify_all();
+  join();
+}
+
+
+void crimson::RunEvery::join() {
+  {
+    Guard l(mtx);
+    if (finishing) return;
+    finishing = true;
+    cv.notify_all();
+  }
   thd.join();
 }
 

diff --git a/support/src/run_every.h b/support/src/run_every.h
index c3499da91efc632be9f6e8f761b28323911494c7..494db92e31657bb4e8a905765b26614ea972a3fb 100644 (file)
--- a/support/src/run_every.h
+++ b/support/src/run_every.h
@@ -1,7 +1,7 @@
 // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
 /*
- * Copyright (C) 2016 Red Hat Inc.
+ * Copyright (C) 2017 Red Hat Inc.
  */
 
 
@@ -61,6 +61,8 @@ namespace crimson {
 
     ~RunEvery();
 
+    void join();
+
   protected:
 
     void run();

diff --git a/support/test/CMakeLists.txt b/support/test/CMakeLists.txt
index addea6c96a996c607345c6139674705142084bb6..1e1ea25cc14e3dcaac1c2bca421a82e8eab025c2 100644 (file)
--- a/support/test/CMakeLists.txt
+++ b/support/test/CMakeLists.txt
@@ -24,6 +24,3 @@ add_executable(dmclock-data-struct-tests ${test_srcs})
 
 target_link_libraries(dmclock-data-struct-tests
   LINK_PRIVATE gtest gtest_main pthread)
-
-add_test(NAME dmclock-data-struct-tests
-  COMMAND $<TARGET_FILE:dmclock-data-struct-tests>)

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index e72810b56aa73feeaaf738922f50fba44d853e13..24da2e7320a5d02343355ee442b825a424bdbf17 100644 (file)
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -1,7 +1,6 @@
 include_directories(../src)
 include_directories(../support/src)
 include_directories(../sim/src)
-include_directories(${BOOST_INCLUDE_DIR})
 
 set(support_srcs ../sim/src/test_dmclock.cc)
 set(test_srcs
@@ -26,10 +25,7 @@ if (TARGET gtest AND TARGET gtest_main)
     $<TARGET_FILE:gtest_main>)
 else()
   target_link_libraries(dmclock-tests
-    LINK_PRIVATE $<TARGET_FILE:dmclock> pthread ${GTEST_LIBRARY} ${GTEST_MAIN_LIBRARY})
+    LINK_PRIVATE $<TARGET_FILE:dmclock> pthread ${GTEST_LIBRARIES} ${GTEST_MAIN_LIBRARIES})
 endif()
   
 add_dependencies(dmclock-tests dmclock)
-
-add_test(NAME dmclock-tests
-  COMMAND $<TARGET_FILE:dmclock-tests>)

diff --git a/test/test_test_client.cc b/test/test_test_client.cc
index 6015cb9bf7bb020a004eaa8022266d1878fd50e7..dd04fd179af8f5a325454447f1eef66be0ab4227 100644 (file)
--- a/test/test_test_client.cc
+++ b/test/test_test_client.cc
@@ -61,6 +61,8 @@ TEST(test_client, full_bore_timing) {
   int milliseconds = (end - start) / std::chrono::milliseconds(1);
   EXPECT_LT(10000, milliseconds) << "timing too fast to be correct";
   EXPECT_GT(12000, milliseconds) << "timing suspiciously slow";
+
+  delete client;
 }
 
 
@@ -120,4 +122,6 @@ TEST(test_client, paused_timing) {
   EXPECT_LT(15000 - 500, milliseconds) << "timing too fast to be correct";
   EXPECT_GT(17000 - 500, milliseconds) << "timing suspiciously slow";
   t.join();
+
+  delete client;
 }