wip

Author: MarcelloPerathoner

CMakeLists.txt

@@ -255,7 +255,7 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
     set(COLOR_FLAG "")
   endif()
   # using GCC
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 ${COLOR_FLAG} -fPIC -ftemplate-depth=1024")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 ${COLOR_FLAG} -fPIC -ftemplate-depth=1024 -fcompare-debug-second")
 
   if(WIN32) # using mingw
     add_dependency_defines(-DWIN32)
@@ -277,6 +277,9 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
   target_link_libraries(osrm-extract wsock32 ws2_32)
 endif()
 
+message(STATUS "CMAKE_CXX_COMPILER: ${CMAKE_CXX_COMPILER}")
+message(STATUS "CMAKE_CXX_FLAGS: ${CMAKE_CXX_FLAGS}")
+
 if(UNIX AND NOT APPLE)
   find_library(RT_LIB rt)
   if (RT_LIB)

cmake/warnings.cmake

@@ -46,9 +46,13 @@ macro (target_no_warning target flag)
     target_add_warning(${target} no-${flag})
 endmacro ()
 
-add_warning(all)
-add_warning(extra)
-add_warning(pedantic)
+if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
+    add_warning(4)
+else()
+    add_warning(all)
+    add_warning(extra)
+    add_warning(pedantic)
+endif()
 add_warning(error) # treat all warnings as errors
 if(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
     add_warning(strict-overflow=1)
@@ -85,4 +89,6 @@ no_warning(restrict)
 no_warning(free-nonheap-object)
 if(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
   no_warning(stringop-overflow)
-endif()
+  no_warning(uninitialized)
+  no_warning(array-bounds)
+endif()

features/support/data.js

@@ -117,6 +117,7 @@ module.exports = function () {
     var node = new OSM.Node(id, this.OSM_USER, this.OSM_TIMESTAMP, this.OSM_UID, lon, lat, {name: name});
     this.OSMDB.addNode(node);
     this.nameNodeHash[name] = node;
+    this.idNodeHash[id] = node;
   };
 
   this.addLocation = (name, lon, lat) => {

include/extractor/area/area_manager.hpp

@@ -32,15 +32,17 @@ class AreaManager
     /** Mutex to protect relations_manager */
     MutexType mutex;
 
+    using relation_ids = std::vector<osmium::object_id_type>;
+
   public:
     AreaManager() { m_assembler_config.debug_level = 0; };
 
     void way(const osmium::Way &way);
     void relation(const osmium::Relation &relation);
     void prepare_for_lookup(extractor::NodeLocationsForWays &node_locations_for_ways);
     bool is_registered_closed_way(osmium::object_id_type way_id) const;
-    ExtractionRelationContainer::RelationIDList get_relations_node(const osmium::Node &) const;
-    ExtractionRelationContainer::RelationIDList get_relations_way(const osmium::Way &) const;
+    relation_ids get_relations_for_node(const osmium::Node &) const;
+    relation_ids get_relations_for_way(const osmium::Way &) const;
 
     void complete_relation(const osmium::Relation &relation);
     void after_way(const osmium::Way &way);

include/extractor/area/area_mesher.hpp

@@ -59,8 +59,8 @@ class AreaMesher
                                          std::set<OsmiumSegment> &vis_map,
                                          const NodeRefSet &entry_points);
 
-    ExtractionRelationContainer::RelationIDList
-    get_relations(const osmium::Area &area, const ExtractionRelationContainer &relations);
+    osmium::object_id_type get_relations(const osmium::Area &area,
+                                         const ExtractionRelationContainer &relations);
 
     std::unordered_multimap<OSMNodeID, OSMWayID> node_id2way_index;
     osmium::object_id_type next_way_id{(1ULL << 34) - 1}; // see: packed_osm_ids.hpp

include/extractor/extraction_relation.hpp

@@ -5,139 +5,218 @@
 
 #include <boost/assert.hpp>
 #include <boost/flyweight.hpp>
+#include <boost/flyweight/flyweight.hpp>
+#include <boost/flyweight/flyweight_fwd.hpp>
 #include <boost/flyweight/no_tracking.hpp>
 
+#include <oneapi/tbb/concurrent_map.h>
+
 #include <osmium/osm/area.hpp>
 #include <osmium/osm/item_type.hpp>
 #include <osmium/osm/node.hpp>
 #include <osmium/osm/object.hpp>
 #include <osmium/osm/relation.hpp>
+#include <osmium/osm/tag.hpp>
 #include <osmium/osm/types.hpp>
 #include <osmium/osm/way.hpp>
 
-#include <oneapi/tbb/concurrent_map.h>
-#include <oneapi/tbb/mutex.h>
-
-#include <osmium/storage/item_stash.hpp>
+#include <algorithm>
+#include <map>
 #include <string>
 #include <vector>
 
+#include "util/log.hpp"
+
 namespace osrm::extractor
 {
 
+namespace
+{
+using string = boost::flyweights::flyweight<std::string, boost::flyweights::no_tracking>;
+}
+
+/**
+ * @brief A copiable RelationMember
+ */
+class RelationMember
+{
+    osmium::object_id_type m_ref;
+    osmium::item_type m_type;
+    string m_role;
+
+  public:
+    RelationMember(osmium::object_id_type id, osmium::item_type type) : m_ref{id}, m_type{type} {};
+    RelationMember(const osmium::RelationMember &o)
+        : m_ref{o.ref()}, m_type{o.type()}, m_role(o.role()){};
+    osmium::object_id_type ref() const noexcept { return m_ref; }
+    osmium::item_type type() const noexcept { return m_type; }
+    const string &role() const noexcept { return m_role; }
+
+    friend bool operator==(const RelationMember &m, const RelationMember &o)
+    {
+        return m.ref() == o.ref() && m.type() == o.type();
+    }
+    friend bool operator<(const RelationMember &m, const RelationMember &o)
+    {
+        return m.ref() < o.ref() || (m.ref() == o.ref() && m.type() < m.type());
+    }
+};
+
+/**
+ * @brief A copiable Relation
+ *
+ * Motivation: osmium entities are almost impossible to use with LUA because they have
+ * no copy constructors. Eg. SOL, some 15 template invocations down, wants to
+ * copy-construct an entity before giving it to LUA. This implementation duck-types an
+ * `osmium::Relation` for SOL/LUA.
+ *
+ * Care has been taken to minimize memory consumption. The alternative to use an
+ * `osmium::stash` was found to bee too heavy on memory.
+ */
+class Relation
+{
+    using members_t = std::vector<RelationMember>;
+    osmium::object_id_type m_id;
+    osmium::object_version_type m_version;
+    std::map<string, string> m_tags;
+    members_t m_members;
+    bool sorted{false};
+
+  public:
+    Relation(const osmium::Relation &o) : m_id{o.id()}, m_version(o.version())
+    {
+        for (const osmium::Tag &tag : o.tags())
+        {
+            m_tags.emplace(tag.key(), tag.value());
+        }
+        for (const osmium::RelationMember &member : o.cmembers())
+        {
+            m_members.emplace_back(member);
+        }
+    };
+
+    osmium::object_id_type id() const noexcept { return m_id; };
+    osmium::item_type type() const noexcept { return osmium::item_type::relation; };
+    osmium::object_version_type version() const noexcept { return m_version; };
+
+    const char *get_value_by_key_default(const char *key, const char *default_value) const
+    {
+        // Nitpick: if somebody searches for "higway", it will be added to the flyweights
+        auto found = m_tags.find(string(key));
+        if (found != m_tags.end())
+            return found->second->c_str();
+        return default_value;
+    };
+
+    const char *get_value_by_key(const char *key) const
+    {
+        return get_value_by_key_default(key, nullptr);
+    }
+
+    template <class T> const char *get_member_role(const T &o)
+    {
+        return get_member_role(RelationMember(o.id(), o.type()));
+    }
+
+    const char *get_member_role(const RelationMember &m)
+    {
+        if (!sorted)
+        {
+            std::sort(m_members.begin(), m_members.end());
+            sorted = true;
+        }
+        auto it = std::lower_bound(m_members.begin(), m_members.end(), m);
+        if (it != m_members.end() && *it == m)
+        {
+            return it->role()->c_str();
+        }
+        return nullptr;
+    }
+
+    const members_t &members() { return m_members; };
+};
+
 // It contains data of all parsed relations for each node/way element
 class ExtractionRelationContainer
 {
-  public:
     using rel_id_type = osmium::object_id_type;
-    using handle_type = osmium::ItemStash::handle_type;
-    using mutex_type = tbb::mutex;
+    using member_id_type = osmium::object_id_type;
+
+    using rel_ids_t = std::vector<rel_id_type>;
+    using parent_map_t = tbb::concurrent_map<member_id_type, rel_ids_t>;
+
+    tbb::concurrent_map<rel_id_type, Relation> relations;
+    std::vector<parent_map_t> parents;
 
-    using RelationIDList = std::vector<rel_id_type>;
-    using id_and_type = std::pair<osmium::object_id_type, osmium::item_type>;
+    rel_ids_t empty_rel_list{};
 
-    using RelationHandleMap = tbb::concurrent_map<rel_id_type, handle_type>;
-    using MemberRelationMap = tbb::concurrent_map<id_and_type, RelationIDList>;
+    parent_map_t &p(osmium::item_type t) { return parents[(size_t)t]; }
+    const parent_map_t &p(osmium::item_type t) const { return parents[(size_t)t]; }
 
-    ExtractionRelationContainer() = default;
+  public:
     ExtractionRelationContainer(ExtractionRelationContainer &&) = delete;
     ExtractionRelationContainer(const ExtractionRelationContainer &) = delete;
+    ExtractionRelationContainer() : parents(4){};
 
     void AddRelation(const osmium::Relation &rel)
     {
-        BOOST_ASSERT(handles.find(rel.id()) == handles.end());
+        BOOST_ASSERT(!relations.contains(rel.id()));
         for (auto const &m : rel.members())
         {
             AddRelationMember(rel.id(), m.ref(), m.type());
         }
-
-        handle_type handle;
-        {
-            mutex_type::scoped_lock lock(mutex);
-            handle = stash.add_item(rel);
-        }
-        handles.emplace(rel.id(), handle);
+        relations.emplace(rel.id(), rel);
     }
 
     void
     AddRelationMember(rel_id_type relation_id, osmium::object_id_type id, osmium::item_type type)
     {
-        parents[id_and_type{id, type}].push_back(relation_id);
+        p(type)[id].push_back(relation_id);
     }
 
-    std::size_t GetRelationsNum() const { return handles.size(); }
+    std::size_t GetRelationsNum() const { return relations.size(); }
 
-    const RelationIDList &_GetRelationsFor(osmium::object_id_type id, osmium::item_type type) const
+    /**
+     * @brief Returns the relations that the given object is a member of.
+     *
+     * @param id The id of the object
+     * @param type The type of the object
+     * @return const rel_ids_t& The list of relation ids
+     */
+    const rel_ids_t &_GetRelationsFor(osmium::object_id_type id, osmium::item_type type) const
     {
         if (type == osmium::item_type::area)
         {
             type = (id & 1) ? osmium::item_type::relation : osmium::item_type::way;
             id /= 2;
         }
-        auto it = parents.find(id_and_type{id, type});
+        assert(type == osmium::item_type::relation || type == osmium::item_type::way ||
+               type == osmium::item_type::node);
+
+        const parent_map_t &parents = p(type);
+
+        util::Log(logINFO) << "size of parents map: " << parents.size();
+
+        auto it = parents.find(id);
         if (it != parents.end())
             return it->second;
 
         return empty_rel_list;
     }
 
-    const RelationIDList &GetRelationsFor(const osmium::OSMObject &o) const
+    template <class T> const rel_ids_t &GetRelationsFor(const T &o) const
     {
         return _GetRelationsFor(o.id(), o.type());
     }
 
-    // Note: non-const because SOL somehow chokes on const.
-    osmium::Relation &GetRelation(rel_id_type rel_id) const
+    const Relation &GetRelation(rel_id_type rel_id) const
     {
-        auto it = handles.find(rel_id);
-        if (it == handles.end())
+        auto it = relations.find(rel_id);
+        if (it == relations.end())
             throw osrm::util::exception("Can't find relation data for " + std::to_string(rel_id));
 
-        return stash.get<osmium::Relation>(it->second);
+        return it->second;
     }
-
-    void reset() { iter = handles.begin(); }
-
-    osmium::memory::Buffer read()
-    {
-        osmium::memory::Buffer buffer(16 * 1024, osmium::memory::Buffer::auto_grow::yes);
-        auto end = handles.end();
-        if (iter == end)
-            return osmium::memory::Buffer();
-        while (iter != end && buffer.written() < 12 * 1024)
-        {
-            buffer.add_item(stash.get_item(iter->second));
-            buffer.commit();
-            ++iter;
-        }
-        return buffer;
-    };
-
-  private:
-    osmium::ItemStash stash;
-    RelationIDList empty_rel_list;
-    RelationHandleMap handles;
-    RelationHandleMap::const_iterator iter;
-    MemberRelationMap parents;
-
-    mutex_type mutex;
-};
-
-/**
- * @brief A copiable RelationMember
- */
-class RelationMember
-{
-    osmium::object_id_type m_ref;
-    osmium::item_type m_type;
-    std::string m_role;
-
-  public:
-    RelationMember(const osmium::RelationMember &o)
-        : m_ref{o.ref()}, m_type{o.type()}, m_role(o.role()){};
-    osmium::object_id_type ref() const noexcept { return m_ref; }
-    osmium::item_type type() const noexcept { return m_type; }
-    const std::string &role() const noexcept { return m_role; }
 };
 
 } // namespace osrm::extractor