diff --git a/CMakeLists.txt b/CMakeLists.txt index c8f4877..857ae03 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,13 +9,9 @@ include(GNUInstallDirs) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) - -# Configure the option whether you want to use the systems own allocater -# DHB_WITH_SYSTEM_ALLOCATOR=ON -# or our own written allocator strategy -# DHB_WITH_SYSTEM_ALLOCATOR=OFF option(DHB_WITH_SYSTEM_ALLOCATOR "Use DHB System Allocator" OFF) option(DHB_WITH_64BIT_IDS "Use 64 bit IDs." OFF) +option(DHB_WARNINGS_AS_ERRORS "Sets warning level high, treats warnings as errors ." OFF) add_library(dhb STATIC) @@ -42,18 +38,21 @@ target_sources(${PROJECT_NAME} # Options check and set definitions # =================================== if (DHB_WITH_SYSTEM_ALLOCATOR) - MESSAGE(STATUS "Using DHB System Allocator.") + MESSAGE(STATUS "DHB uses system allocator.") target_compile_definitions(dhb PUBLIC -DDHB_SYSTEM_ALLOCATOR) endif() if (DHB_WITH_64BIT_IDS) - MESSAGE(STATUS "Using 64 bit IDs.") + MESSAGE(STATUS "DHB uses 64 bit vertex IDs.") target_compile_definitions(dhb PUBLIC -DDHB_64BIT_IDS) else() - MESSAGE(STATUS "Using 32 bit IDs.") + MESSAGE(STATUS "DHB uses 32 bit vertex IDs.") endif() -set_property(TARGET dhb PROPERTY POSITION_INDEPENDENT_CODE ON) +if (DHB_WARNINGS_AS_ERRORS) + MESSAGE(STATUS "DHB is extra pedantic, throws errors on warnings.") + target_compile_options(dhb PRIVATE -Werror -Wall -Wextra -Wpedantic -Wmaybe-uninitialized) +endif() find_package(OpenMP) # Thanks to NetworKit for the following OpenMP code looking or OpenMP in the @@ -95,9 +94,6 @@ if(NOT OpenMP_FOUND) endif() target_link_libraries(${PROJECT_NAME} PUBLIC OpenMP::OpenMP_CXX) -# Be extra pedantic about warnings! -target_compile_options(${PROJECT_NAME} PRIVATE -Wall -Wextra -Wpedantic -Werror) - target_include_directories(${PROJECT_NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src @@ -128,8 +124,10 @@ set_target_properties(${PROJECT_NAME}::${PROJECT_NAME} PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include" ) -set_target_properties(${PROJECT_NAME} PROPERTIES PUBLIC_HEADER "${public_headers}") -set_target_properties(${PROJECT_NAME} PROPERTIES DEBUG_POSTFIX "d") +set_target_properties(dhb PROPERTIES CXX_STANDARD 17) +set_target_properties(dhb PROPERTIES POSITION_INDEPENDENT_CODE ON) +set_target_properties(dhb PROPERTIES DEBUG_POSTFIX "d") +set_target_properties(dhb PROPERTIES PUBLIC_HEADER "${public_headers}") # =================================== # Make a Configuration Package @@ -192,11 +190,16 @@ if (DHB_BUILD_TESTS) test/graph_io.h test/graph_io.cpp test/matrix_test.cpp - test/vec.cpp + test/vec_test.cpp test/block.cpp - test/submatrix.cpp + test/submatrix_test.cpp ) + if (DHB_WARNINGS_AS_ERRORS) + MESSAGE(STATUS "DHB test is extra pedantic, throws errors on warnings.") + target_compile_options(dhb_test PRIVATE -Werror -Wall -Wextra -Wpedantic -Wmaybe-uninitialized) + endif() + target_link_libraries(dhb_test PRIVATE dhb Catch2::Catch2WithMain OpenMP::OpenMP_CXX) install( diff --git a/include/dhb/batcher.h b/include/dhb/batcher.h index 99e4250..20c0058 100644 --- a/include/dhb/batcher.h +++ b/include/dhb/batcher.h @@ -69,7 +69,7 @@ template class BatchParallelizer { public: template void operator()(Iterator begin, Iterator end, GetSourceF&& get_source_f, Cmp cmp, F func) { - int const t_count = omp_get_max_threads(); + uint32_t const t_count = omp_get_max_threads(); size_t const n = end - begin; if (t_count == 1 || n < t_count) { for (auto it = begin; it != end; ++it) @@ -90,7 +90,7 @@ template class BatchParallelizer { template void apply(Iterator begin, Iterator end, K key, F func) { - size_t const t_count = omp_get_max_threads(); + uint32_t const t_count = omp_get_max_threads(); size_t const n = end - begin; if (t_count == 1 || n < t_count) { for (auto it = begin; it != end; ++it) @@ -104,7 +104,7 @@ template class BatchParallelizer { #pragma omp parallel num_threads(t_count) { size_t const t = omp_get_thread_num(); - assert(omp_get_num_threads() == t_count); + assert(uint32_t(omp_get_num_threads()) == t_count); auto counts_of_thread = [&](int ct) -> uint64_t* { return &m_batch_counts[ct * (t_count + 1)]; @@ -156,7 +156,6 @@ template class BatchParallelizer { auto j_end = ot_counts[t + 1]; for (size_t j = j_begin; j < j_end; ++j) { auto i = m_batch_slots[j]; - auto edge = *(begin + i); func(*(begin + i)); } local_count += j_end - j_begin; @@ -165,7 +164,7 @@ template class BatchParallelizer { } template void distribute(Iterator begin, Iterator end, K key) { - int const t_count = omp_get_num_threads(); + uint32_t const t_count = omp_get_num_threads(); size_t const n = end - begin; if (t_count == 1 || n < t_count) { return; @@ -177,7 +176,7 @@ template class BatchParallelizer { m_batch_slots.resize(n); } - auto t = omp_get_thread_num(); + uint32_t t = omp_get_thread_num(); auto counts_of_thread = [&](int ct) -> uint64_t* { return &m_batch_counts[ct * (t_count + 1)]; @@ -192,7 +191,7 @@ template class BatchParallelizer { // First, perform a local counting sort to sort updates according to associated threads. auto t_counts = counts_of_thread(t); - for (int at = 0; at < t_count; ++at) + for (uint32_t at = 0; at < t_count; ++at) t_counts[at] = 0; for (size_t i = i_begin; i < i_end; ++i) { @@ -202,7 +201,7 @@ template class BatchParallelizer { } uint64_t psum = 0; - for (int at = 0; at < t_count; ++at) { + for (uint32_t at = 0; at < t_count; ++at) { psum += t_counts[at]; t_counts[at] = i_begin + psum; } @@ -223,7 +222,7 @@ template class BatchParallelizer { } template void map(Iterator begin, Iterator end, F func) { - int const t_count = omp_get_num_threads(); + uint32_t const t_count = omp_get_num_threads(); size_t const n = end - begin; if (t_count == 1 || n < t_count) { #pragma omp master @@ -243,13 +242,12 @@ template class BatchParallelizer { }; uint64_t local_count = 0; - for (int ot = 0; ot < t_count; ++ot) { + for (uint32_t ot = 0; ot < t_count; ++ot) { auto ot_counts = counts_of_thread(ot); auto j_begin = ot_counts[t]; auto j_end = ot_counts[t + 1]; for (size_t j = j_begin; j < j_end; ++j) { auto i = m_batch_slots[j]; - auto edge = *(begin + i); func(*(begin + i)); } local_count += j_end - j_begin; diff --git a/include/dhb/block.h b/include/dhb/block.h index 2064ce7..b01b7d2 100644 --- a/include/dhb/block.h +++ b/include/dhb/block.h @@ -5,27 +5,31 @@ #include #include +#include #include -#include #include +#ifndef DHB_SYSTEM_ALLOCATOR +#include +#endif + namespace dhb { class BlockHandle; class BlockArray; using index_type = size_t; -inline index_type illegalIndex() { return static_cast(-1); }; -inline index_type tombstoneIndex() { return static_cast(-2); }; +inline index_type illegalIndex() { return static_cast(-1); } +inline index_type tombstoneIndex() { return static_cast(-2); } // TODO: This could take the entry size into account. -inline bool uses_htab(size_t bsize) { - const size_t cache_line = 64; - // Each entry is typically 16 bytes or so. - // Preliminary experiments how that using the hash index is only useful if the block becomes - // larger than several cache lines (approx. 16 or so). - // Hence, the following is a reasonably good heuristic: - return 16 * bsize > 16 * cache_line; +inline bool uses_htab(size_t const bsize) { + size_t constexpr cache_line = 64; + // Each entry is typically 16 bytes or so. Preliminary experiments show that + // using the hash index is only useful if the block becomes larger than + // several cache lines (approx. 16 or so). Hence, the following is a + // reasonably good heuristic: + return 16u * bsize > 16u * cache_line; } // Taken from https://stackoverflow.com/a/12996028. @@ -416,7 +420,7 @@ template class BlockState { if (uses_htab(m_bsize)) { index_type* htab = m_htab; auto h = hash_node(v); - index_type j; + index_type j = 0u; index_type ts = illegalIndex(); for (index_type i = 0; true; ++i) { if (i == m_bsize) { @@ -437,8 +441,9 @@ template class BlockState { } // If we did hit a tombstone, insert at the tombstone. - if (ts != illegalIndex()) + if (ts != illegalIndex()) { j = ts; + } // Insert into the adjacency list. auto i = m_degree++; @@ -521,7 +526,7 @@ template class BlockState { } else { // Find the index. index_type iv; - for (iv = 0; iv < m_degree; ++iv) + for (iv = 0u; iv < m_degree; ++iv) if (m_entries[iv].vertex == v) break; @@ -530,7 +535,7 @@ template class BlockState { return false; // Swap with the last entry. - if (iv + 1 != m_degree) + if (iv + 1u != m_degree) m_entries[iv] = m_entries[m_degree - 1]; // Remove the last entry. diff --git a/include/dhb/dynamic_hashed_blocks.h b/include/dhb/dynamic_hashed_blocks.h index 1d38290..a9ebda6 100644 --- a/include/dhb/dynamic_hashed_blocks.h +++ b/include/dhb/dynamic_hashed_blocks.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -55,7 +56,6 @@ template struct Matrix { BlockState new_block{new_bhandle, state}; auto result = new_block.insert(v, ed); - auto old_block = std::move(m_graph->m_vertices[m_source]); auto old_bhandle = m_graph->m_handles[m_source]; m_graph->m_vertices[m_source] = std::move(new_block); m_graph->m_handles[m_source] = new_bhandle; @@ -174,7 +174,6 @@ template struct Matrix { auto new_bhandle = m_manager->allocate_block(degree); BlockState new_block{new_bhandle, associated_block}; - auto old_block = std::move(m_vertices[u]); auto old_bhandle = m_handles[u]; m_vertices[u] = std::move(new_block); m_handles[u] = new_bhandle; diff --git a/include/dhb/vec.h b/include/dhb/vec.h index 6b9c090..1f3be69 100644 --- a/include/dhb/vec.h +++ b/include/dhb/vec.h @@ -59,7 +59,6 @@ template struct Vec { BlockState new_block{new_bhandle, state}; auto result = new_block.insert(u, ed); - auto old_block = std::move(m_state); auto old_bhandle = m_handle; m_state = std::move(new_block); m_handle = new_bhandle; diff --git a/test/matrix_test.cpp b/test/matrix_test.cpp index 21b887d..4085467 100644 --- a/test/matrix_test.cpp +++ b/test/matrix_test.cpp @@ -272,7 +272,6 @@ TEST_CASE("Matrix") { Edges new_edges{e89_14, e89_8, e89_13_update}; auto cmp = [](Edge const& a, Edge const& b) { return a.source < b.source; }; - auto key = [](Edge e) { return e.source; }; auto fun = [&](Edge e) { std::tuple::iterator, bool> insertion_result = m.neighbors(e.source).insert(e.target.vertex, e.target.data); @@ -306,7 +305,6 @@ TEST_CASE("Matrix") { Matrix m(graph::vertex_count(edges)); auto cmp = [](Edge const& a, Edge const& b) { return a.source < b.source; }; - auto key = [](Edge e) { return e.source; }; auto fun = [&](Edge e) { m.neighbors(e.source).insert(e.target.vertex, e.target.data); }; auto get_edge_f = [](Edge const& e) { return e.source; }; BatchParallelizer par; @@ -318,7 +316,6 @@ TEST_CASE("Matrix") { Edges new_edges{e89_14, e89_8}; auto cmp2 = [](Edge const& a, Edge const& b) { return a.source < b.source; }; - auto key2 = [](Edge e) { return e.source; }; par(new_edges.begin(), new_edges.end(), std::move(get_edge_f), std::move(cmp2), [&](Edge e) { m.neighbors(e.source).insert(e.target.vertex, e.target.data); }); @@ -351,7 +348,6 @@ TEST_CASE("Matrix") { Edges new_edges{e89_14, e89_8}; auto cmp2 = [](Edge const& a, Edge const& b) { return a.source < b.source; }; - auto key2 = [](Edge e) { return e.source; }; par(new_edges.begin(), new_edges.end(), std::move(get_edge_f), std::move(cmp2), [&](Edge e) { m.neighbors(e.source).insert(e.target.vertex, e.target.data); }); @@ -372,9 +368,7 @@ TEST_CASE("Matrix") { Matrix m(graph::vertex_count(edges)); - auto cmp = [](Edge const& a, Edge const& b) { return a.source < b.source; }; auto key = [](Edge e) { return e.source; }; - auto fun = [&](Edge e) { m.neighbors(e.source).insert(e.target.vertex, e.target.data); }; auto get_edge_f = [](Edge const& e) { return e.source; }; BatchParallelizer par; @@ -401,7 +395,6 @@ TEST_CASE("Matrix") { Edges new_edges{e89_14, e89_8}; auto cmp2 = [](Edge const& a, Edge const& b) { return a.source < b.source; }; - auto key2 = [](Edge e) { return e.source; }; par(new_edges.begin(), new_edges.end(), std::move(get_edge_f), std::move(cmp2), [&](Edge e) { m.neighbors(e.source).insert(e.target.vertex, e.target.data); }); @@ -423,7 +416,6 @@ TEST_CASE("Matrix") { Matrix m(graph::vertex_count(edges)); auto cmp = [](Edge const& a, Edge const& b) { return a.source < b.source; }; - auto key = [](Edge e) { return e.source; }; auto get_edge_f = [](Edge const& e) { return e.source; }; BatchParallelizer par; @@ -437,9 +429,6 @@ TEST_CASE("Matrix") { Edges new_edges{e89_14, e89_8, e89_13_update}; - auto cmp2 = [](Edge const& a, Edge const& b) { return a.source < b.source; }; - auto key2 = [](Edge e) { return e.source; }; - par(new_edges.begin(), new_edges.end(), std::move(get_edge_f), std::move(cmp), [&](Edge e) { std::tuple::iterator, bool> insertion_result = m.neighbors(e.source).insert(e.target.vertex, e.target.data); diff --git a/test/submatrix.cpp b/test/submatrix_test.cpp similarity index 95% rename from test/submatrix.cpp rename to test/submatrix_test.cpp index 76a3ddc..84bd479 100644 --- a/test/submatrix.cpp +++ b/test/submatrix_test.cpp @@ -15,9 +15,9 @@ class TestMatrix { // [ 13 | 4 | 0 ] // clang-format off dhb::Edges edges_a = { - {0, Tar{0, ED{1.0}}}, {0, Tar{1, ED{3.0}}}, {0, Tar{2, ED{0.0}}}, - {1, Tar{0, ED{4.0}}}, {1, Tar{1, ED{5.0}}}, {1, Tar{2, ED{8.0}}}, - {2, Tar{0, ED{13.0}}}, {2, Tar{1, ED{4.0}}}, {2, Tar{2, ED{0.0}}}, + {0, Tar{0, ED{1.0, 0}}}, {0, Tar{1, ED{3.0, 0}}}, {0, Tar{2, ED{0.0, 0}}}, + {1, Tar{0, ED{4.0, 0}}}, {1, Tar{1, ED{5.0, 0}}}, {1, Tar{2, ED{8.0, 0}}}, + {2, Tar{0, ED{13.0, 0}}}, {2, Tar{1, ED{4.0, 0}}}, {2, Tar{2, ED{0.0, 0}}}, }; // clang-format on diff --git a/test/vec.cpp b/test/vec_test.cpp similarity index 86% rename from test/vec.cpp rename to test/vec_test.cpp index 91c7285..0fc32f9 100644 --- a/test/vec.cpp +++ b/test/vec_test.cpp @@ -2,7 +2,7 @@ #include TEST_CASE("Vec") { - dhb::Vec v; + dhb::Vec v; REQUIRE(std::get<1>(v.insert(2, 1))); REQUIRE(std::get<1>(v.insert(3, 1))); @@ -11,11 +11,11 @@ TEST_CASE("Vec") { REQUIRE(std::get<1>(v.insert(11, 1))); SECTION("iterate") { - std::array indices{2, 3, 5, 7, 11}; + std::array indices{2, 3, 5, 7, 11}; bool entries_are_equal = std::equal(v.begin(), v.end(), indices.begin(), indices.end(), - [](auto ent, int idx) -> bool { return ent.vertex() == idx; }); + [](auto ent, uint32_t idx) -> bool { return ent.vertex() == idx; }); CHECK(entries_are_equal); bool all_edge_data_is_1 = @@ -63,11 +63,11 @@ TEST_CASE("Vec") { SECTION("erase") { v.erase(v.find(11)); v.erase(v.find(5)); - std::array indices{2, 3, 7}; + std::array indices{2, 3, 7}; bool erase_took_place = std::equal(v.begin(), v.end(), indices.begin(), indices.end(), - [](auto ent, int idx) -> bool { return ent.vertex() == idx; }); + [](auto ent, uint32_t idx) -> bool { return ent.vertex() == idx; }); CHECK(erase_took_place); }