Refactor file writing in OSRM contract

include/contractor/contractor.hpp

@@ -65,16 +65,9 @@ class Contractor
                        std::vector<EdgeWeight> &&node_weights,
                        std::vector<bool> &is_core_node,
                        std::vector<float> &inout_node_levels) const;
-    void WriteCoreNodeMarker(std::vector<bool> &&is_core_node) const;
-    void WriteContractedGraph(unsigned number_of_edge_based_nodes,
-                              util::DeallocatingVector<QueryEdge> contracted_edge_list);
 
   private:
     ContractorConfig config;
-
-    EdgeID LoadEdgeExpandedGraph(const ContractorConfig &config,
-                                 std::vector<extractor::EdgeBasedEdge> &edge_based_edge_list,
-                                 std::vector<EdgeWeight> &node_weights);
 };
 }
 }

include/contractor/files.hpp

@@ -14,6 +14,27 @@ namespace contractor
 {
 namespace files
 {
+// reads .osrm.core
+template <typename CoreVectorT>
+void readCoreMarker(const boost::filesystem::path &path, CoreVectorT &is_core_node)
+{
+    static_assert(util::is_view_or_vector<bool, CoreVectorT>::value,
+                  "is_core_node must be a vector");
+    storage::io::FileReader reader(path, storage::io::FileReader::VerifyFingerprint);
+
+    storage::serialization::read(reader, is_core_node);
+}
+
+// writes .osrm.core
+template <typename CoreVectorT>
+void writeCoreMarker(const boost::filesystem::path &path, const CoreVectorT &is_core_node)
+{
+    static_assert(util::is_view_or_vector<bool, CoreVectorT>::value,
+                  "is_core_node must be a vector");
+    storage::io::FileWriter writer(path, storage::io::FileWriter::GenerateFingerprint);
+
+    storage::serialization::write(writer, is_core_node);
+}
 
 // reads .osrm.hsgr file
 template <typename QueryGraphT>

include/contractor/graph_contractor.hpp

@@ -91,8 +91,8 @@ class GraphContractor
 
     GraphContractor(int nodes,
                     std::vector<ContractorEdge> edges,
-                    std::vector<float> &&node_levels_,
+                    std::vector<float> node_levels_,
-                    std::vector<EdgeWeight> &&node_weights_);
+                    std::vector<EdgeWeight> node_weights_);
 
     /* Flush all data from the contraction to disc and reorder stuff for better locality */
     void FlushDataAndRebuildContractorGraph(ThreadDataContainer &thread_data_list,
@@ -101,12 +101,14 @@ class GraphContractor
 
     void Run(double core_factor = 1.0);
 
-    void GetCoreMarker(std::vector<bool> &out_is_core_node);
+    std::vector<bool> GetCoreMarker();
 
-    void GetNodeLevels(std::vector<float> &out_node_levels);
+    std::vector<float> GetNodeLevels();
 
-    template <class Edge> inline void GetEdges(util::DeallocatingVector<Edge> &edges)
+    template <class Edge> inline util::DeallocatingVector<Edge> GetEdges()
     {
+        util::DeallocatingVector<Edge> edges;
+
         util::UnbufferedLog log;
         log << "Getting edges of minimized graph ";
         util::Percent p(log, contractor_graph->GetNumberOfNodes());
@@ -161,6 +163,13 @@ class GraphContractor
 
         edges.append(external_edge_list.begin(), external_edge_list.end());
         external_edge_list.clear();
+
+        // sort and remove duplicates
+        tbb::parallel_sort(edges.begin(), edges.end());
+        auto new_end = std::unique(edges.begin(), edges.end());
+        edges.resize(new_end - edges.begin());
+
+        return edges;
     }
 
   private:

include/storage/serialization.hpp

@@ -104,7 +104,47 @@ template <typename T> void write(io::FileWriter &writer, const util::vector_view
 {
     const auto count = data.size();
     writer.WriteElementCount64(count);
-    return writer.WriteFrom(data.data(), count);
+    writer.WriteFrom(data.data(), count);
+}
+
+template <> inline void read<bool>(io::FileReader &reader, util::vector_view<bool> &data)
+{
+    const auto count = reader.ReadElementCount64();
+    BOOST_ASSERT(data.size() == count);
+    for (const auto index : util::irange<std::uint64_t>(0, count))
+    {
+        data[index] = reader.ReadOne<bool>();
+    }
+}
+
+template <> inline void write<bool>(io::FileWriter &writer, const util::vector_view<bool> &data)
+{
+    const auto count = data.size();
+    writer.WriteElementCount64(count);
+    for (const auto index : util::irange<std::uint64_t>(0, count))
+    {
+        writer.WriteOne<bool>(data[index]);
+    }
+}
+
+template <> inline void read<bool>(io::FileReader &reader, std::vector<bool> &data)
+{
+    const auto count = reader.ReadElementCount64();
+    BOOST_ASSERT(data.size() == count);
+    for (const auto index : util::irange<std::uint64_t>(0, count))
+    {
+        data[index] = reader.ReadOne<bool>();
+    }
+}
+
+template <> inline void write<bool>(io::FileWriter &writer, const std::vector<bool> &data)
+{
+    const auto count = data.size();
+    writer.WriteElementCount64(count);
+    for (const auto index : util::irange<std::uint64_t>(0, count))
+    {
+        writer.WriteOne<bool>(data[index]);
+    }
 }
 }
 }

include/util/vector_view.hpp

@@ -143,8 +143,33 @@ template <> class vector_view<bool>
     unsigned *m_ptr;
     std::size_t m_size;
 
+    static constexpr std::size_t UNSIGNED_BITS = CHAR_BIT * sizeof(unsigned);
+
   public:
     using value_type = bool;
+    struct reference
+    {
+        reference &operator=(bool value)
+        {
+            *m_ptr = (*m_ptr & ~mask) | (static_cast<unsigned>(value) * mask);
+            return *this;
+        }
+
+        operator bool() const { return (*m_ptr) & mask; }
+
+        bool operator==(const reference &other) const
+        {
+            return other.m_ptr == m_ptr && other.mask == mask;
+        }
+
+        friend std::ostream &operator<<(std::ostream &os, const reference &rhs)
+        {
+            return os << static_cast<bool>(rhs);
+        }
+
+        unsigned *m_ptr;
+        const unsigned mask;
+    };
 
     vector_view() : m_ptr(nullptr), m_size(0) {}
 
@@ -153,8 +178,8 @@ template <> class vector_view<bool>
     bool at(const std::size_t index) const
     {
         BOOST_ASSERT_MSG(index < m_size, "invalid size");
-        const std::size_t bucket = index / (CHAR_BIT * sizeof(unsigned));
+        const std::size_t bucket = index / UNSIGNED_BITS;
-        const unsigned offset = index % (CHAR_BIT * sizeof(unsigned));
+        const unsigned offset = index % UNSIGNED_BITS;
         return m_ptr[bucket] & (1u << offset);
     }
 
@@ -166,6 +191,14 @@ template <> class vector_view<bool>
 
     bool operator[](const unsigned index) const { return at(index); }
 
+    reference operator[](const unsigned index)
+    {
+        BOOST_ASSERT(index < m_size);
+        const std::size_t bucket = index / UNSIGNED_BITS;
+        const unsigned offset = index % UNSIGNED_BITS;
+        return reference{m_ptr + bucket, 1u << offset};
+    }
+
     template <typename T> friend void swap(vector_view<T> &, vector_view<T> &) noexcept;
 };
 
@@ -186,6 +219,15 @@ template <typename DataT, storage::Ownership Ownership>
 using ViewOrVector = typename std::conditional<Ownership == storage::Ownership::View,
                                                vector_view<DataT>,
                                                InternalOrExternalVector<DataT, Ownership>>::type;
+
+// We can use this for compile time assertions
+template <typename ValueT, typename VectorT>
+struct is_view_or_vector
+    : std::integral_constant<bool,
+                             std::is_same<std::vector<ValueT>, VectorT>::value ||
+                                 std::is_same<util::vector_view<ValueT>, VectorT>::value>
+{
+};
 }
 }
 

src/contractor/contractor.cpp

@@ -77,20 +77,27 @@ int Contractor::Run()
                                          std::move(node_levels),
                                          std::move(node_weights));
         graph_contractor.Run(config.core_factor);
-        graph_contractor.GetEdges(contracted_edge_list);
+
-        graph_contractor.GetCoreMarker(is_core_node);
+        contracted_edge_list = graph_contractor.GetEdges<QueryEdge>();
-        graph_contractor.GetNodeLevels(node_levels);
+        is_core_node = graph_contractor.GetCoreMarker();
+        node_levels = graph_contractor.GetNodeLevels();
     }
     TIMER_STOP(contraction);
 
     util::Log() << "Contraction took " << TIMER_SEC(contraction) << " sec";
 
-    WriteContractedGraph(max_edge_id, std::move(contracted_edge_list));
+    {
-    WriteCoreNodeMarker(std::move(is_core_node));
+        RangebasedCRC32 crc32_calculator;
+        const unsigned checksum = crc32_calculator(contracted_edge_list);
+
+        files::writeGraph(config.graph_output_path,
+                          checksum,
+                          QueryGraph{max_edge_id + 1, std::move(contracted_edge_list)});
+    }
+
+    files::writeCoreMarker(config.core_output_path, is_core_node);
     if (!config.use_cached_priority)
     {
-        std::vector<float> out_node_levels(std::move(node_levels));
-
         files::writeLevels(config.level_output_path, node_levels);
     }
 
@@ -103,38 +110,5 @@ int Contractor::Run()
     return 0;
 }
 
-void Contractor::WriteCoreNodeMarker(std::vector<bool> &&in_is_core_node) const
-{
-    std::vector<bool> is_core_node(std::move(in_is_core_node));
-    std::vector<char> unpacked_bool_flags(std::move(is_core_node.size()));
-    for (auto i = 0u; i < is_core_node.size(); ++i)
-    {
-        unpacked_bool_flags[i] = is_core_node[i] ? 1 : 0;
-    }
-
-    storage::io::FileWriter core_marker_output_file(config.core_output_path,
-                                                    storage::io::FileWriter::GenerateFingerprint);
-
-    const std::size_t count = unpacked_bool_flags.size();
-    core_marker_output_file.WriteElementCount64(count);
-    core_marker_output_file.WriteFrom(unpacked_bool_flags.data(), count);
-}
-
-void Contractor::WriteContractedGraph(unsigned max_node_id,
-                                      util::DeallocatingVector<QueryEdge> contracted_edge_list)
-{
-    // Sorting contracted edges in a way that the static query graph can read some in in-place.
-    tbb::parallel_sort(contracted_edge_list.begin(), contracted_edge_list.end());
-    auto new_end = std::unique(contracted_edge_list.begin(), contracted_edge_list.end());
-    contracted_edge_list.resize(new_end - contracted_edge_list.begin());
-
-    RangebasedCRC32 crc32_calculator;
-    const unsigned checksum = crc32_calculator(contracted_edge_list);
-
-    QueryGraph query_graph{max_node_id + 1, contracted_edge_list};
-
-    files::writeGraph(config.graph_output_path, checksum, query_graph);
-}
-
 } // namespace contractor
 } // namespace osrm

src/contractor/graph_contractor.cpp

@@ -12,8 +12,8 @@ GraphContractor::GraphContractor(int nodes, std::vector<ContractorEdge> input_ed
 
 GraphContractor::GraphContractor(int nodes,
                                  std::vector<ContractorEdge> edges,
-                                 std::vector<float> &&node_levels_,
+                                 std::vector<float> node_levels_,
-                                 std::vector<EdgeWeight> &&node_weights_)
+                                 std::vector<EdgeWeight> node_weights_)
     : node_levels(std::move(node_levels_)), node_weights(std::move(node_weights_))
 {
     tbb::parallel_sort(edges.begin(), edges.end());
@@ -427,15 +427,11 @@ void GraphContractor::Run(double core_factor)
     thread_data_list.data.clear();
 }
 
-void GraphContractor::GetCoreMarker(std::vector<bool> &out_is_core_node)
+// Can only be called once because it invalides the marker
-{
+std::vector<bool> GraphContractor::GetCoreMarker() { return std::move(is_core_node); }
-    out_is_core_node.swap(is_core_node);
-}
 
-void GraphContractor::GetNodeLevels(std::vector<float> &out_node_levels)
+// Can only be called once because it invalides the node levels
-{
+std::vector<float> GraphContractor::GetNodeLevels() { return std::move(node_levels); }
-    out_node_levels.swap(node_levels);
-}
 
 float GraphContractor::EvaluateNodePriority(ContractorThreadData *const data,
                                             const NodeDepth node_depth,

src/storage/storage.cpp

@@ -809,36 +809,12 @@ void Storage::PopulateData(const DataLayout &layout, char *memory_ptr)
 
     if (boost::filesystem::exists(config.core_data_path))
     {
-        io::FileReader core_marker_file(config.core_data_path, io::FileReader::VerifyFingerprint);
+        auto core_marker_ptr =
-        const auto number_of_core_markers = core_marker_file.ReadElementCount64();
+            layout.GetBlockPtr<unsigned, true>(memory_ptr, storage::DataLayout::CH_CORE_MARKER);
+        util::vector_view<bool> is_core_node(
+            core_marker_ptr, layout.num_entries[storage::DataLayout::CH_CORE_MARKER]);
 
-        // load core markers
+        contractor::files::readCoreMarker(config.core_data_path, is_core_node);
-        std::vector<char> unpacked_core_markers(number_of_core_markers);
-        core_marker_file.ReadInto(unpacked_core_markers.data(), number_of_core_markers);
-
-        const auto core_marker_ptr =
-            layout.GetBlockPtr<unsigned, true>(memory_ptr, DataLayout::CH_CORE_MARKER);
-
-        for (auto i = 0u; i < number_of_core_markers; ++i)
-        {
-            BOOST_ASSERT(unpacked_core_markers[i] == 0 || unpacked_core_markers[i] == 1);
-
-            if (unpacked_core_markers[i] == 1)
-            {
-                const unsigned bucket = i / 32;
-                const unsigned offset = i % 32;
-                const unsigned value = [&] {
-                    unsigned return_value = 0;
-                    if (0 != offset)
-                    {
-                        return_value = core_marker_ptr[bucket];
-                    }
-                    return return_value;
-                }();
-
-                core_marker_ptr[bucket] = (value | (1u << offset));
-            }
-        }
     }
 
     // load profile properties

unit_tests/util/vector_view.cpp

@@ -0,0 +1,67 @@
+#include "util/vector_view.hpp"
+#include "util/typedefs.hpp"
+
+#include <boost/range/adaptor/reversed.hpp>
+#include <boost/range/iterator_range.hpp>
+#include <boost/test/test_case_template.hpp>
+#include <boost/test/unit_test.hpp>
+
+#include <algorithm>
+#include <numeric>
+#include <random>
+
+BOOST_AUTO_TEST_SUITE(vector_view_test)
+
+using namespace osrm;
+using namespace osrm::util;
+
+BOOST_AUTO_TEST_CASE(rw_short)
+{
+    std::size_t num_elements = 1000;
+    std::unique_ptr<char[]> data = std::make_unique<char[]>(sizeof(std::uint16_t) * num_elements);
+    util::vector_view<std::uint16_t> view(reinterpret_cast<std::uint16_t *>(data.get()),
+                                          num_elements);
+    std::vector<std::uint16_t> reference;
+
+    std::mt19937 rng;
+    rng.seed(1337);
+    std::uniform_int_distribution<std::mt19937::result_type> dist(0, (1UL << 16));
+
+    for (std::size_t i = 0; i < num_elements; i++)
+    {
+        auto r = dist(rng);
+        view[i] = r;
+        reference.push_back(r);
+    }
+
+    for (std::size_t i = 0; i < num_elements; i++)
+    {
+        BOOST_CHECK_EQUAL(view[i], reference[i]);
+    }
+}
+
+BOOST_AUTO_TEST_CASE(rw_bool)
+{
+    std::size_t num_elements = 1000;
+    std::unique_ptr<char[]> data = std::make_unique<char[]>(num_elements / sizeof(std::uint32_t));
+    util::vector_view<bool> view(reinterpret_cast<std::uint32_t *>(data.get()), num_elements);
+    std::vector<bool> reference;
+
+    std::mt19937 rng;
+    rng.seed(1337);
+    std::uniform_int_distribution<std::mt19937::result_type> dist(0, 2);
+
+    for (std::size_t i = 0; i < num_elements; i++)
+    {
+        auto r = dist(rng);
+        view[i] = r;
+        reference.push_back(r);
+    }
+
+    for (std::size_t i = 0; i < num_elements; i++)
+    {
+        BOOST_CHECK_EQUAL(view[i], reference[i]);
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()