Open-Harbor/osrm-backend
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 3 Wiki Activity

Fix MLD level mask generation to support 64-bit masks. (#6123)

Browse Source 
The generation of level masks for compactly storing partition cells
supports sizes that can be stored in 64 bits.

The current implementation fails if the total bit sum is 64 bits
exactly. A bit shift mechanism is used that is undefined when the
shift size is equal to the bit size of the underlying type. This
generates an incorrect mask value.

We fix this by adding a special case for a 64 bit offset. Given this
code is called at most |level| times, there will be no effect on
performance. We also update the assertions to reflect 64 bit masks
are now supported.
This commit is contained in:
Michael Bell 2021-09-21 18:09:38 +01:00 committed by GitHub
parent 8e100ea1a0
commit f1f96166c5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 68 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -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

15
include/partitioner/multi_level_partition.hpp
View File

@ -186,11 +186,12 @@ template <storage::Ownership Ownership> class MultiLevelPartitionImpl final
auto bits = static_cast<std::uint64_t>(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 <storage::Ownership Ownership> 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;
});

57
unit_tests/partitioner/multi_level_partition.cpp
View File

@ -1,6 +1,8 @@
#include <boost/numeric/conversion/cast.hpp>
#include <boost/test/unit_test.hpp>
#include "util/exception.hpp"
#include "util/for_each_indexed.hpp"
#include <util/integer_range.hpp>
#include <util/msb.hpp>
@ -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<size_t> level_cells = {NUM_PARTITIONS, 258451, 16310, 534};
std::vector<std::vector<CellID>> levels(level_cells.size(),
std::vector<CellID>(level_cells[0]));
std::vector<uint32_t> levels_to_num_cells(level_cells.size());
const auto set_level_cells = [&](size_t level, auto const num_cells) {
for (auto val : util::irange<size_t>(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<size_t> level_cells = {NUM_PARTITIONS, 262144, 16384, 1024};
std::vector<std::vector<CellID>> levels(level_cells.size(),
std::vector<CellID>(level_cells[0]));
std::vector<uint32_t> levels_to_num_cells(level_cells.size());
const auto set_level_cells = [&](size_t level, auto const num_cells) {
for (auto val : util::irange<size_t>(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()