From 5c612d7e32a07a01b3b97f9cf58f2a3cef0e2e73 Mon Sep 17 00:00:00 2001 From: "J. Eric Ivancich" Date: Tue, 6 Jun 2017 15:01:02 -0400 Subject: [PATCH] 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 --- CMakeLists.txt | 31 ++++++--------- cmake/modules/Findboost.cmake | 15 -------- cmake/modules/Findgtest.cmake | 48 ------------------------ sim/CMakeLists.txt | 8 ++++ sim/src/sim_server.h | 2 + sim/src/simulate.h | 11 ++++++ sim/src/test_dmclock_main.cc | 2 + src/CMakeLists.txt | 9 ++++- src/dmclock_server.h | 54 +++++++++++++++++---------- support/CMakeLists.txt | 1 - support/src/indirect_intrusive_heap.h | 2 +- support/src/run_every.cc | 17 ++++++--- support/src/run_every.h | 4 +- support/test/CMakeLists.txt | 3 -- test/CMakeLists.txt | 6 +-- test/test_test_client.cc | 4 ++ 16 files changed, 97 insertions(+), 120 deletions(-) delete mode 100644 cmake/modules/Findboost.cmake delete mode 100644 cmake/modules/Findgtest.cmake delete mode 100644 support/CMakeLists.txt diff --git a/CMakeLists.txt b/CMakeLists.txt index 428863dc496df..9444ea1e96afd 100644 --- 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 $) +add_test(NAME dmclock-data-struct-tests + COMMAND $) diff --git a/cmake/modules/Findboost.cmake b/cmake/modules/Findboost.cmake deleted file mode 100644 index 4f0dfd052f082..0000000000000 --- 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 index bfe0980e4ed7a..0000000000000 --- 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 febd4f0ab6f82..c088d2f1cd7c9 100644 --- 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 a61cc3204e4e9..e318d6e90ac32 100644 --- 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 18e752d8a354f..a967d004b6dcf 100644 --- 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 c3ba1e18fbd6d..668869b8d72fb 100644 --- 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 691e64cce4389..7178f266e2a41 100644 --- 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 65013063fa715..8aaad39772648 100644 --- 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 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 index 552439ebc59da..0000000000000 --- 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 b6075bda22f92..6342e29b16370 100644 --- 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 258baaa74c041..cab414109a9dc 100644 --- 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 - #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 c3499da91efc6..494db92e31657 100644 --- 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 addea6c96a996..1e1ea25cc14e3 100644 --- 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 $) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e72810b56aa73..24da2e7320a5d 100644 --- 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) $) else() target_link_libraries(dmclock-tests - LINK_PRIVATE $ pthread ${GTEST_LIBRARY} ${GTEST_MAIN_LIBRARY}) + LINK_PRIVATE $ pthread ${GTEST_LIBRARIES} ${GTEST_MAIN_LIBRARIES}) endif() add_dependencies(dmclock-tests dmclock) - -add_test(NAME dmclock-tests - COMMAND $) diff --git a/test/test_test_client.cc b/test/test_test_client.cc index 6015cb9bf7bb0..dd04fd179af8f 100644 --- 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; } -- 2.39.5