Packed vector cleanups and unit test

include/util/packed_vector.hpp

@@ -3,6 +3,7 @@
 
 #include "util/typedefs.hpp"
 
+#include <cmath>
 #include <vector>
 
 namespace osrm
@@ -14,15 +15,21 @@ const constexpr std::size_t BITSIZE = 33;
 const constexpr std::size_t ELEMSIZE = 64;
 const constexpr std::size_t PACKSIZE = BITSIZE * ELEMSIZE;
 
+/**
+ * Since OSM node IDs are (at the time of writing) not quite yet overflowing 32 bits, and
+ * will predictably be containable within 33 bits for a long time, the following packs
+ * 64-bit OSM IDs as 33-bit numbers within a 64-bit vector.
+ */
 class PackedVector
 {
   public:
     PackedVector() = default;
 
-    void insert(OSMNodeID &node_id)
+    void insert(OSMNodeID incoming_node_id)
     {
+        std::uint64_t node_id = static_cast<std::uint64_t>(incoming_node_id);
         // mask incoming values, just in case they are > bitsize
-        std::uint64_t incoming_mask = static_cast<std::uint64_t>(pow(2, BITSIZE)) - 1;
+        const std::uint64_t incoming_mask = static_cast<std::uint64_t>(pow(2, BITSIZE)) - 1;
         node_id = node_id & incoming_mask;
 
         const std::size_t available = (PACKSIZE - BITSIZE * num_elements) % ELEMSIZE;
@@ -37,14 +44,14 @@ class PackedVector
         {
             // insert ID somewhere in the middle of this element; ID can be contained
             // entirely within one element
-            std::uint64_t shifted = node_id << (available - BITSIZE);
+            const std::uint64_t shifted = node_id << (available - BITSIZE);
             vec.back() = vec.back() | shifted;
         }
         else
         {
             // ID will be split between the end of this element and the beginning
             // of the next element
-            std::uint64_t left = node_id >> (BITSIZE - available);
+            const std::uint64_t left = node_id >> (BITSIZE - available);
             vec.back() = vec.back() | left;
 
             std::uint64_t right = node_id << (ELEMSIZE - (BITSIZE - available));
@@ -56,46 +63,50 @@ class PackedVector
 
     OSMNodeID retrieve(const std::size_t &a_index) const
     {
-        // TODO check if OOB
+        BOOST_ASSERT(a_index < num_elements);
 
         const std::size_t pack_group = trunc(a_index / ELEMSIZE);
-        const std::size_t pack_index = (a_index + ELEMSIZE) % ELEMSIZE;       // ?
+        const std::size_t pack_index = (a_index + ELEMSIZE) % ELEMSIZE;
         const std::size_t left_index = (PACKSIZE - BITSIZE * pack_index) % ELEMSIZE;
 
         const bool back_half = pack_index >= BITSIZE;
-        std::size_t index = pack_group * BITSIZE + trunc(pack_index / BITSIZE) + trunc((pack_index - back_half) / 2);
+        const std::size_t index = pack_group * BITSIZE + trunc(pack_index / BITSIZE) +
+                                  trunc((pack_index - back_half) / 2);
 
-        std::uint64_t elem = vec.at(index);
+        BOOST_ASSERT(index < vec.size());
+        const std::uint64_t elem = vec.at(index);
 
         if (left_index == 0)
         {
             // ID is at the far left side of this element
-            return elem >> (ELEMSIZE - BITSIZE);
+            return static_cast<OSMNodeID>(elem >> (ELEMSIZE - BITSIZE));
         }
         else if (left_index >= BITSIZE)
         {
             // ID is entirely contained within this element
-            std::uint64_t at_right = elem >> (left_index - BITSIZE);
+            const std::uint64_t at_right = elem >> (left_index - BITSIZE);
-            std::uint64_t left_mask = static_cast<std::uint64_t>(pow(2, BITSIZE)) - 1;
+            const std::uint64_t left_mask = static_cast<std::uint64_t>(pow(2, BITSIZE)) - 1;
-            return at_right & left_mask;
+            return static_cast<OSMNodeID>(at_right & left_mask);
         }
         else
         {
             // ID is split between this and the next element
-            std::uint64_t left_mask = static_cast<std::uint64_t>(pow(2, left_index)) - 1;
+            const std::uint64_t left_mask = static_cast<std::uint64_t>(pow(2, left_index)) - 1;
-            std::uint64_t left_side = (elem & left_mask) << (BITSIZE - left_index);
+            const std::uint64_t left_side = (elem & left_mask) << (BITSIZE - left_index);
 
-            // TODO check OOB
+            BOOST_ASSERT(index < vec.size() - 1);
-            std::uint64_t next_elem = vec.at(index + 1);
+            const std::uint64_t next_elem = vec.at(index + 1);
 
-            std::uint64_t right_side = next_elem >> (ELEMSIZE - (BITSIZE - left_index));
+            const std::uint64_t right_side = next_elem >> (ELEMSIZE - (BITSIZE - left_index));
-            return left_side | right_side;
+            return static_cast<OSMNodeID>(left_side | right_side);
         }
     }
 
   private:
-    std::vector<OSMNodeID> vec;
+    std::vector<std::uint64_t> vec;
     std::size_t num_elements = 0;
 };
+}
+}
 
 #endif /* PACKED_VECTOR_HPP */

unit_tests/util/packed_vector.cpp

@@ -0,0 +1,34 @@
+#include "util/packed_vector.hpp"
+#include "util/typedefs.hpp"
+
+#include <boost/test/unit_test.hpp>
+#include <boost/test/test_case_template.hpp>
+
+BOOST_AUTO_TEST_SUITE(packed_vector_test)
+
+using namespace osrm;
+using namespace osrm::util;
+
+// Verify that the packed vector behaves as expected
+BOOST_AUTO_TEST_CASE(insert_and_retrieve_packed_test)
+{
+    PackedVector packed_ids;
+    std::vector<OSMNodeID> original_ids;
+
+    const constexpr std::size_t num_test_cases = 399;
+
+    for (std::size_t i = 0; i < num_test_cases; i++)
+    {
+        OSMNodeID r = static_cast<OSMNodeID>(rand() % 2147483647); // max 33-bit uint
+
+        packed_ids.insert(r);
+        original_ids.push_back(r);
+    }
+
+    for (std::size_t i = 0; i < num_test_cases; i++)
+    {
+        BOOST_CHECK_EQUAL(original_ids.at(i), packed_ids.retrieve(i));
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()