diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bcf36c0af3..c0038417bb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - FIXED: Fixed Boost link flags in pkg-config file. [#6083](https://github.com/Project-OSRM/osrm-backend/pull/6083) - Routing: - FIXED: Fix generation of inefficient MLD partitions [#6084](https://github.com/Project-OSRM/osrm-backend/pull/6084) + - FIXED: Fix MLD level mask generation to support 64-bit masks. [#6123](https://github.com/Project-OSRM/osrm-backend/pull/6123) # 5.25.0 - Changes from 5.24.0 diff --git a/include/partitioner/multi_level_partition.hpp b/include/partitioner/multi_level_partition.hpp index 855aa5ae2f0..d83f9a341aa 100644 --- a/include/partitioner/multi_level_partition.hpp +++ b/include/partitioner/multi_level_partition.hpp @@ -186,11 +186,12 @@ template class MultiLevelPartitionImpl final auto bits = static_cast(std::ceil(std::log2(num_cells + 1))); offsets[lidx++] = sum_bits; sum_bits += bits; - if (sum_bits > 64) + if (sum_bits > NUM_PARTITION_BITS) { throw util::exception( "Can't pack the partition information at level " + std::to_string(lidx) + - " into a 64bit integer. Would require " + std::to_string(sum_bits) + " bits."); + " into a " + std::to_string(NUM_PARTITION_BITS) + + "bit integer. Would require " + std::to_string(sum_bits) + " bits."); } } // sentinel @@ -211,11 +212,15 @@ template class MultiLevelPartitionImpl final [&](const auto offset, const auto next_offset) { // create mask that has `bits` ones at its LSBs. // 000011 - BOOST_ASSERT(offset < NUM_PARTITION_BITS); + BOOST_ASSERT(offset <= NUM_PARTITION_BITS); PartitionID mask = (1ULL << offset) - 1ULL; // 001111 - BOOST_ASSERT(next_offset < NUM_PARTITION_BITS); - PartitionID next_mask = (1ULL << next_offset) - 1ULL; + BOOST_ASSERT(next_offset <= NUM_PARTITION_BITS); + // Check offset for shift overflow. Offsets are strictly increasing, + // so we only need the check on the last mask. + PartitionID next_mask = next_offset == NUM_PARTITION_BITS + ? -1ULL + : (1ULL << next_offset) - 1ULL; // 001100 masks[lidx++] = next_mask ^ mask; }); diff --git a/unit_tests/partitioner/multi_level_partition.cpp b/unit_tests/partitioner/multi_level_partition.cpp index d25664525e9..7fc7acd1452 100644 --- a/unit_tests/partitioner/multi_level_partition.cpp +++ b/unit_tests/partitioner/multi_level_partition.cpp @@ -1,6 +1,8 @@ #include #include +#include "util/exception.hpp" +#include "util/for_each_indexed.hpp" #include #include @@ -230,4 +232,59 @@ BOOST_AUTO_TEST_CASE(large_cell_number) } } +BOOST_AUTO_TEST_CASE(cell_64_bits) +{ + // bits = ceil(log2(2458529 + 1)) + ceil(log2(258451 + 1)) + ceil(log2(16310 + 1)) + + // ceil(log2(534 + 1)) + // = 22 + 18 + 14 + 10 + // = 64 + const size_t NUM_PARTITIONS = 2458529; + const std::vector level_cells = {NUM_PARTITIONS, 258451, 16310, 534}; + std::vector> levels(level_cells.size(), + std::vector(level_cells[0])); + std::vector levels_to_num_cells(level_cells.size()); + + const auto set_level_cells = [&](size_t level, auto const num_cells) { + for (auto val : util::irange(0ULL, NUM_PARTITIONS)) + { + levels[level][val] = std::min(val, num_cells - 1); + } + levels_to_num_cells[level] = num_cells; + }; + util::for_each_indexed(level_cells.cbegin(), level_cells.cend(), set_level_cells); + + MultiLevelPartition mlp{levels, levels_to_num_cells}; + + BOOST_REQUIRE_EQUAL(mlp.GetNumberOfCells(1), level_cells[0]); + BOOST_REQUIRE_EQUAL(mlp.GetNumberOfCells(2), level_cells[1]); + BOOST_REQUIRE_EQUAL(mlp.GetNumberOfCells(3), level_cells[2]); + BOOST_REQUIRE_EQUAL(mlp.GetNumberOfCells(4), level_cells[3]); +} + +BOOST_AUTO_TEST_CASE(cell_overflow_bits) +{ + // bits = ceil(log2(4194304 + 1)) + ceil(log2(262144 + 1)) + ceil(log2(16384 + 1)) + + // ceil(log2(1024 + 1)) + // = 23 + 19 + 15 + 11 + // = 68 + const size_t NUM_PARTITIONS = 4194304; + const std::vector level_cells = {NUM_PARTITIONS, 262144, 16384, 1024}; + std::vector> levels(level_cells.size(), + std::vector(level_cells[0])); + std::vector levels_to_num_cells(level_cells.size()); + + const auto set_level_cells = [&](size_t level, auto const num_cells) { + for (auto val : util::irange(0ULL, NUM_PARTITIONS)) + { + levels[level][val] = std::min(val, num_cells - 1); + } + levels_to_num_cells[level] = num_cells; + }; + util::for_each_indexed(level_cells.cbegin(), level_cells.cend(), set_level_cells); + + BOOST_REQUIRE_EXCEPTION(MultiLevelPartition(levels, levels_to_num_cells), + util::exception, + [](auto) { return true; }); +} + BOOST_AUTO_TEST_SUITE_END()