From 8f1de40ca1e621662775d78a417e82273bf1909c Mon Sep 17 00:00:00 2001 From: Daniel Salwasser Date: Tue, 16 Jul 2024 11:53:58 +0200 Subject: [PATCH] refactor(compressed-graph): cleanup code --- .../shm_variable_length_codec_benchmark.cc | 6 +- .../compressed_neighborhoods.h | 18 ++-- .../{ => graph-compression}/varint_codec.cc | 2 +- .../{ => graph-compression}/varint_codec.h | 0 .../varint_run_length_codec.h | 86 +++++++++---------- .../varint_stream_codec.h | 32 +++---- kaminpar-common/math.h | 6 +- kaminpar-shm/context_io.cc | 2 +- tests/common/varint_codec_test.cc | 2 +- tests/common/varint_run_length_codec_test.cc | 2 +- tests/common/varint_stream_codec_test.cc | 2 +- .../distributed_compressed_graph_test.cc | 15 ++-- 12 files changed, 85 insertions(+), 88 deletions(-) rename kaminpar-common/{ => graph-compression}/varint_codec.cc (92%) rename kaminpar-common/{ => graph-compression}/varint_codec.h (100%) rename kaminpar-common/{ => graph-compression}/varint_run_length_codec.h (79%) rename kaminpar-common/{ => graph-compression}/varint_stream_codec.h (90%) diff --git a/apps/benchmarks/shm_variable_length_codec_benchmark.cc b/apps/benchmarks/shm_variable_length_codec_benchmark.cc index fc5bc1d0..746adc97 100644 --- a/apps/benchmarks/shm_variable_length_codec_benchmark.cc +++ b/apps/benchmarks/shm_variable_length_codec_benchmark.cc @@ -13,11 +13,11 @@ #include "kaminpar-cli/CLI11.h" #include "kaminpar-common/console_io.h" +#include "kaminpar-common/graph-compression/varint_codec.h" +#include "kaminpar-common/graph-compression/varint_run_length_codec.h" +#include "kaminpar-common/graph-compression/varint_stream_codec.h" #include "kaminpar-common/logger.h" #include "kaminpar-common/timer.h" -#include "kaminpar-common/varint_codec.h" -#include "kaminpar-common/varint_run_length_codec.h" -#include "kaminpar-common/varint_stream_codec.h" using namespace kaminpar; diff --git a/kaminpar-common/graph-compression/compressed_neighborhoods.h b/kaminpar-common/graph-compression/compressed_neighborhoods.h index fc4754ca..e6e78c5a 100644 --- a/kaminpar-common/graph-compression/compressed_neighborhoods.h +++ b/kaminpar-common/graph-compression/compressed_neighborhoods.h @@ -10,11 +10,11 @@ #include "kaminpar-common/constexpr_utils.h" #include "kaminpar-common/datastructures/compact_static_array.h" #include "kaminpar-common/datastructures/static_array.h" +#include "kaminpar-common/graph-compression/varint_codec.h" +#include "kaminpar-common/graph-compression/varint_run_length_codec.h" +#include "kaminpar-common/graph-compression/varint_stream_codec.h" #include "kaminpar-common/math.h" #include "kaminpar-common/ranges.h" -#include "kaminpar-common/varint_codec.h" -#include "kaminpar-common/varint_run_length_codec.h" -#include "kaminpar-common/varint_stream_codec.h" namespace kaminpar { @@ -562,7 +562,7 @@ template class Compresse } else { for (NodeID part = 0; part < part_count; ++part) { const bool stop = iterate_part(part); - if (stop) { + if (stop) [[unlikely]] { return; } } @@ -586,11 +586,11 @@ template class Compresse const bool stop = decode_intervals( data, edge, prev_edge_weight, std::forward(l) ); - if (stop) { + if (stop) [[unlikely]] { return true; } - if (edge == max_edge) { + if (edge == max_edge) [[unlikely]] { return false; } } @@ -649,7 +649,7 @@ template class Compresse invoke_caller(cur_left_extreme + j); } else { const bool stop = invoke_caller(cur_left_extreme + j); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -703,7 +703,7 @@ template class Compresse invoke_caller(first_adjacent_node); } else { const bool stop = invoke_caller(first_adjacent_node); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -741,7 +741,7 @@ template class Compresse invoke_caller(adjacent_node); } else { const bool stop = invoke_caller(adjacent_node); - if (stop) { + if (stop) [[unlikely]] { return true; } } diff --git a/kaminpar-common/varint_codec.cc b/kaminpar-common/graph-compression/varint_codec.cc similarity index 92% rename from kaminpar-common/varint_codec.cc rename to kaminpar-common/graph-compression/varint_codec.cc index d2bfed3c..0905c592 100644 --- a/kaminpar-common/varint_codec.cc +++ b/kaminpar-common/graph-compression/varint_codec.cc @@ -5,7 +5,7 @@ * @author: Daniel Salwasser * @date: 26.12.2023 ******************************************************************************/ -#include "kaminpar-common/varint_codec.h" +#include "kaminpar-common/graph-compression/varint_codec.h" namespace kaminpar { diff --git a/kaminpar-common/varint_codec.h b/kaminpar-common/graph-compression/varint_codec.h similarity index 100% rename from kaminpar-common/varint_codec.h rename to kaminpar-common/graph-compression/varint_codec.h diff --git a/kaminpar-common/varint_run_length_codec.h b/kaminpar-common/graph-compression/varint_run_length_codec.h similarity index 79% rename from kaminpar-common/varint_run_length_codec.h rename to kaminpar-common/graph-compression/varint_run_length_codec.h index 8e545fe1..17c7b84b 100644 --- a/kaminpar-common/varint_run_length_codec.h +++ b/kaminpar-common/graph-compression/varint_run_length_codec.h @@ -7,8 +7,8 @@ ******************************************************************************/ #pragma once +#include #include -#include #include #include @@ -143,7 +143,7 @@ template class VarIntRunLengthDecoder { decode32(run_length, run_size, std::forward(l)); } else { const bool stop = decode32(run_length, run_size, std::forward(l)); - if (stop) { + if (stop) [[unlikely]] { return; } } @@ -157,7 +157,7 @@ template class VarIntRunLengthDecoder { decode64(run_length, run_size, std::forward(l)); } else { const bool stop = decode64(run_length, run_size, std::forward(l)); - if (stop) { + if (stop) [[unlikely]] { return; } } @@ -171,19 +171,19 @@ template class VarIntRunLengthDecoder { template bool decode32(const std::uint8_t run_length, const std::uint8_t run_size, Lambda &&l) { - constexpr bool non_stoppable = std::is_void_v>; + constexpr bool kNonStoppable = std::is_void_v>; switch (run_size) { case 1: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint32_t value = static_cast(*_ptr); + const std::uint32_t value = static_cast(*_ptr); _ptr += 1; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -191,14 +191,14 @@ template class VarIntRunLengthDecoder { break; case 2: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint32_t value = *((std::uint16_t *)_ptr); + const std::uint32_t value = *((std::uint16_t *)_ptr); _ptr += 2; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -206,14 +206,14 @@ template class VarIntRunLengthDecoder { break; case 3: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint32_t value = *((std::uint32_t *)_ptr) & 0xFFFFFF; + const std::uint32_t value = *((std::uint32_t *)_ptr) & 0xFFFFFF; _ptr += 3; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -221,21 +221,21 @@ template class VarIntRunLengthDecoder { break; case 4: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint32_t value = *((std::uint32_t *)_ptr); + const std::uint32_t value = *((std::uint32_t *)_ptr); _ptr += 4; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } } break; default: - throw std::runtime_error("unexpected run size"); + __builtin_unreachable(); } return false; @@ -243,19 +243,19 @@ template class VarIntRunLengthDecoder { template bool decode64(const std::uint8_t run_length, const std::uint8_t run_size, Lambda &&l) { - constexpr bool non_stoppable = std::is_void_v>; + constexpr bool kNonStoppable = std::is_void_v>; switch (run_size) { case 1: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint64_t value = static_cast(*_ptr); + const std::uint64_t value = static_cast(*_ptr); _ptr += 1; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -263,14 +263,14 @@ template class VarIntRunLengthDecoder { break; case 2: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint64_t value = *((std::uint16_t *)_ptr); + const std::uint64_t value = *((std::uint16_t *)_ptr); _ptr += 2; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -278,14 +278,14 @@ template class VarIntRunLengthDecoder { break; case 3: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint64_t value = *((std::uint32_t *)_ptr) & 0xFFFFFF; + const std::uint64_t value = *((std::uint32_t *)_ptr) & 0xFFFFFF; _ptr += 3; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -293,14 +293,14 @@ template class VarIntRunLengthDecoder { break; case 4: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint64_t value = *((std::uint32_t *)_ptr); + const std::uint64_t value = *((std::uint32_t *)_ptr); _ptr += 4; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -308,14 +308,14 @@ template class VarIntRunLengthDecoder { break; case 5: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint64_t value = *((std::uint64_t *)_ptr) & 0xFFFFFFFFFF; + const std::uint64_t value = *((std::uint64_t *)_ptr) & 0xFFFFFFFFFF; _ptr += 5; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -323,14 +323,14 @@ template class VarIntRunLengthDecoder { break; case 6: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint64_t value = *((std::uint64_t *)_ptr) & 0xFFFFFFFFFFFF; + const std::uint64_t value = *((std::uint64_t *)_ptr) & 0xFFFFFFFFFFFF; _ptr += 6; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -338,14 +338,14 @@ template class VarIntRunLengthDecoder { break; case 7: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint64_t value = *((std::uint64_t *)_ptr) & 0xFFFFFFFFFFFFFF; + const std::uint64_t value = *((std::uint64_t *)_ptr) & 0xFFFFFFFFFFFFFF; _ptr += 7; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } @@ -353,21 +353,21 @@ template class VarIntRunLengthDecoder { break; case 8: for (std::uint8_t i = 0; i < run_length; ++i) { - std::uint64_t value = *((std::uint64_t *)_ptr); + const std::uint64_t value = *((std::uint64_t *)_ptr); _ptr += 8; - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(value); } else { const bool stop = l(value); - if (stop) { + if (stop) [[unlikely]] { return true; } } } break; default: - throw std::runtime_error("unexpected run size"); + __builtin_unreachable(); } return false; diff --git a/kaminpar-common/varint_stream_codec.h b/kaminpar-common/graph-compression/varint_stream_codec.h similarity index 90% rename from kaminpar-common/varint_stream_codec.h rename to kaminpar-common/graph-compression/varint_stream_codec.h index 23712e60..0a0b3c58 100644 --- a/kaminpar-common/varint_stream_codec.h +++ b/kaminpar-common/graph-compression/varint_stream_codec.h @@ -8,12 +8,12 @@ #pragma once #include +#include #include #include #include "kaminpar-common/constexpr_utils.h" -#include "kaminpar-common/varint_codec.h" namespace kaminpar { @@ -189,7 +189,7 @@ template class VarIntStreamDecoder { * parameter of type Int. */ template void decode(Lambda &&l) { - constexpr bool non_stoppable = std::is_void_v>; + constexpr bool kNonStoppable = std::is_void_v>; for (std::size_t i = 0; i < _control_bytes; ++i) { const std::uint8_t control_byte = _control_bytes_ptr[i]; @@ -201,25 +201,25 @@ template class VarIntStreamDecoder { const std::uint8_t *shuffle_mask = kShuffleTable[control_byte].data(); data = _mm_shuffle_epi8(data, *(const __m128i *)shuffle_mask); - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(_mm_extract_epi32(data, 0)); l(_mm_extract_epi32(data, 1)); l(_mm_extract_epi32(data, 2)); l(_mm_extract_epi32(data, 3)); } else { - if (l(_mm_extract_epi32(data, 0))) { + if (l(_mm_extract_epi32(data, 0))) [[unlikely]] { return; } - if (l(_mm_extract_epi32(data, 1))) { + if (l(_mm_extract_epi32(data, 1))) [[unlikely]] { return; } - if (l(_mm_extract_epi32(data, 2))) { + if (l(_mm_extract_epi32(data, 2))) [[unlikely]] { return; } - if (l(_mm_extract_epi32(data, 3))) { + if (l(_mm_extract_epi32(data, 3))) [[unlikely]] { return; } } @@ -233,10 +233,10 @@ template class VarIntStreamDecoder { __m128i data = _mm_loadu_si128((const __m128i *)_data_ptr); data = _mm_shuffle_epi8(data, *(const __m128i *)shuffle_mask); - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(_mm_extract_epi32(data, 0)); } else { - if (l(_mm_extract_epi32(data, 0))) { + if (l(_mm_extract_epi32(data, 0))) [[unlikely]] { return; } } @@ -249,15 +249,15 @@ template class VarIntStreamDecoder { __m128i data = _mm_loadu_si128((const __m128i *)_data_ptr); data = _mm_shuffle_epi8(data, *(const __m128i *)shuffle_mask); - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(_mm_extract_epi32(data, 0)); l(_mm_extract_epi32(data, 1)); } else { - if (l(_mm_extract_epi32(data, 0))) { + if (l(_mm_extract_epi32(data, 0))) [[unlikely]] { return; } - if (l(_mm_extract_epi32(data, 1))) { + if (l(_mm_extract_epi32(data, 1))) [[unlikely]] { return; } } @@ -270,20 +270,20 @@ template class VarIntStreamDecoder { __m128i data = _mm_loadu_si128((const __m128i *)_data_ptr); data = _mm_shuffle_epi8(data, *(const __m128i *)shuffle_mask); - if constexpr (non_stoppable) { + if constexpr (kNonStoppable) { l(_mm_extract_epi32(data, 0)); l(_mm_extract_epi32(data, 1)); l(_mm_extract_epi32(data, 2)); } else { - if (l(_mm_extract_epi32(data, 0))) { + if (l(_mm_extract_epi32(data, 0))) [[unlikely]] { return; } - if (l(_mm_extract_epi32(data, 1))) { + if (l(_mm_extract_epi32(data, 1))) [[unlikely]] { return; } - if (l(_mm_extract_epi32(data, 2))) { + if (l(_mm_extract_epi32(data, 2))) [[unlikely]] { return; } } diff --git a/kaminpar-common/math.h b/kaminpar-common/math.h index 461f7273..a403d907 100644 --- a/kaminpar-common/math.h +++ b/kaminpar-common/math.h @@ -52,7 +52,7 @@ template constexpr std::size_t abs_diff(const Int * @return The ceiling of x divided by y. */ template constexpr Int1 div_ceil(const Int1 x, const Int2 y) { - return 1 + ((x - 1) / y); + return x / y + (x % y != 0); } template bool is_square(const Int value) { @@ -224,8 +224,8 @@ template double find_mean(const Container &container) { } template -auto find_min_mean_max(const Container &container) - -> std::tuple { +auto find_min_mean_max(const Container &container +) -> std::tuple { return std::make_tuple(find_min(container), find_mean(container), find_max(container)); } diff --git a/kaminpar-shm/context_io.cc b/kaminpar-shm/context_io.cc index 7e3c4e5f..63106ecc 100644 --- a/kaminpar-shm/context_io.cc +++ b/kaminpar-shm/context_io.cc @@ -15,9 +15,9 @@ #include "kaminpar-common/asserting_cast.h" #include "kaminpar-common/console_io.h" +#include "kaminpar-common/graph-compression/varint_codec.h" #include "kaminpar-common/random.h" #include "kaminpar-common/strutils.h" -#include "kaminpar-common/varint_codec.h" namespace kaminpar::shm { using namespace std::string_literals; diff --git a/tests/common/varint_codec_test.cc b/tests/common/varint_codec_test.cc index e5680f71..d39becb0 100644 --- a/tests/common/varint_codec_test.cc +++ b/tests/common/varint_codec_test.cc @@ -1,6 +1,6 @@ #include -#include "kaminpar-common/varint_codec.h" +#include "kaminpar-common/graph-compression/varint_codec.h" using namespace kaminpar; diff --git a/tests/common/varint_run_length_codec_test.cc b/tests/common/varint_run_length_codec_test.cc index a5e30aa4..6b044cad 100644 --- a/tests/common/varint_run_length_codec_test.cc +++ b/tests/common/varint_run_length_codec_test.cc @@ -1,6 +1,6 @@ #include -#include "kaminpar-common/varint_run_length_codec.h" +#include "kaminpar-common/graph-compression/varint_run_length_codec.h" using namespace kaminpar; diff --git a/tests/common/varint_stream_codec_test.cc b/tests/common/varint_stream_codec_test.cc index bc60d75e..f7dcf6f0 100644 --- a/tests/common/varint_stream_codec_test.cc +++ b/tests/common/varint_stream_codec_test.cc @@ -1,6 +1,6 @@ #include -#include "kaminpar-common/varint_stream_codec.h" +#include "kaminpar-common/graph-compression/varint_stream_codec.h" using namespace kaminpar; diff --git a/tests/dist/datastructures/distributed_compressed_graph_test.cc b/tests/dist/datastructures/distributed_compressed_graph_test.cc index 39f10319..ac1a2f1f 100644 --- a/tests/dist/datastructures/distributed_compressed_graph_test.cc +++ b/tests/dist/datastructures/distributed_compressed_graph_test.cc @@ -27,11 +27,12 @@ namespace kaminpar::dist { -template static bool operator==(const IotaRange &a, const IotaRange &b) { +template +[[nodiscard]] static bool operator==(const IotaRange &a, const IotaRange &b) { return a.begin() == b.begin() && a.end() == b.end(); }; -DistributedCompressedGraph compress(const DistributedCSRGraph &graph) { +[[nodiscard]] DistributedCompressedGraph compress(const DistributedCSRGraph &graph) { const mpi::PEID size = mpi::get_comm_size(graph.communicator()); const mpi::PEID rank = mpi::get_comm_rank(graph.communicator()); @@ -42,7 +43,7 @@ DistributedCompressedGraph compress(const DistributedCSRGraph &graph) { graph.edge_distribution().begin(), graph.edge_distribution().end() ); - graph::GhostNodeMapper mapper(rank, node_distribution); + CompactGhostNodeMappingBuilder mapper(rank, node_distribution); CompressedNeighborhoodsBuilder builder( graph.n(), graph.m(), graph.is_edge_weighted() ); @@ -62,7 +63,7 @@ DistributedCompressedGraph compress(const DistributedCSRGraph &graph) { if (graph.is_owned_node(adjacent_node)) { neighbourhood.emplace_back(adjacent_node, edge_weight); } else { - const NodeID original_adjacent_node = graph.local_to_global_node(adjacent_node); + const GlobalNodeID original_adjacent_node = graph.local_to_global_node(adjacent_node); neighbourhood.emplace_back(mapper.new_ghost_node(original_adjacent_node), edge_weight); } }); @@ -82,16 +83,12 @@ DistributedCompressedGraph compress(const DistributedCSRGraph &graph) { }); } - auto [global_to_ghost, ghost_to_global, ghost_owner] = mapper.finalize(); - DistributedCompressedGraph compressed_graph( std::move(node_distribution), std::move(edge_distribution), builder.build(), std::move(node_weights), - std::move(ghost_owner), - std::move(ghost_to_global), - std::move(global_to_ghost), + mapper.finalize(), graph.sorted(), graph.communicator() );