Remove RelationMemberWrap and avoid data copying

include/extractor/extraction_relation.hpp

@@ -1,7 +1,9 @@
 #ifndef EXTRACTION_RELATION_HPP
 #define EXTRACTION_RELATION_HPP
 
-#include "util/osm_id_typed.hpp"
+#include <osmium/osm/relation.hpp>
+
+#include <boost/assert.hpp>
 
 #include <string>
 #include <unordered_map>
@@ -15,6 +17,15 @@ namespace extractor
 struct ExtractionRelation
 {
     using AttributesMap = std::unordered_map<std::string, std::string>;
+    using OsmIDTyped = std::pair<osmium::object_id_type, osmium::item_type>;
+
+    struct OsmIDTypedHash
+    {
+        std::size_t operator()(const OsmIDTyped &id) const
+        {
+            return id.first ^ (static_cast<std::uint64_t>(id.second) << 56);
+        }
+    };
 
     ExtractionRelation() : is_restriction(false) {}
 
@@ -26,10 +37,13 @@ struct ExtractionRelation
 
     bool IsRestriction() const { return is_restriction; }
 
-    AttributesMap &GetMember(util::OsmIDTyped id) { return values[id.Hash()]; }
+    AttributesMap &GetMember(const osmium::RelationMember &member)
+    {
+        return values[OsmIDTyped(member.ref(), member.type())];
+    }
 
     bool is_restriction;
-    std::unordered_map<util::OsmIDTyped::HashType, AttributesMap> values;
+    std::unordered_map<OsmIDTyped, AttributesMap, OsmIDTypedHash> values;
 };
 
 // It contains data of all parsed relations for each node/way element
@@ -37,6 +51,7 @@ class ExtractionRelationContainer
 {
   public:
     using AttributesMap = ExtractionRelation::AttributesMap;
+    using OsmIDTyped = ExtractionRelation::OsmIDTyped;
     using RelationList = std::vector<AttributesMap>;
 
     void AddRelation(const ExtractionRelation &rel)
@@ -46,9 +61,9 @@ class ExtractionRelationContainer
             data[it.first].push_back(it.second);
     }
 
-    const RelationList &Get(const util::OsmIDTyped &id) const
+    const RelationList &Get(const OsmIDTyped &id) const
     {
-        const auto it = data.find(id.Hash());
+        const auto it = data.find(id);
         if (it != data.end())
             return it->second;
 
@@ -57,8 +72,7 @@ class ExtractionRelationContainer
     }
 
   private:
-    // TODO: need to store more common data
+    std::unordered_map<OsmIDTyped, RelationList, ExtractionRelation::OsmIDTypedHash> data;
-    std::unordered_map<util::OsmIDTyped::HashType, RelationList> data;
 };
 
 } // namespace extractor

include/util/osm_id_typed.hpp

@@ -1,46 +0,0 @@
-#ifndef OSM_ID_TYPED_HPP
-#define OSM_ID_TYPED_HPP
-
-#include "typedefs.hpp"
-
-namespace osrm
-{
-namespace util
-{
-
-class OsmIDTyped
-{
-  public:
-    enum class Type : uint8_t
-    {
-        Unknown = 0,
-        Node,
-        Way,
-        Relation
-    };
-
-    using HashType = std::uint64_t;
-
-    OsmIDTyped(std::uint64_t id_, Type type_) : id(id_), type(type_)
-    {
-        // check if type value not above type size bound
-        BOOST_ASSERT(id_ < (std::uint64_t(1) << 56));
-    }
-
-    bool operator==(const OsmIDTyped &other) { return (id == other.id && type == other.type); }
-    bool operator!=(const OsmIDTyped &other) { return (id != other.id || type != other.type); }
-
-    inline HashType Hash() const { return (std::uint64_t(id) | std::uint64_t(type) << 56); }
-
-    std::uint64_t GetID() const { return id; }
-    Type GetType() const { return type; }
-
-  private:
-    std::uint64_t id : 56;
-    Type type;
-};
-
-} // namespace util
-} // namespace osrm
-
-#endif

src/extractor/scripting_environment_lua.cpp

@@ -340,62 +340,13 @@ void Sol2ScriptingEnvironment::InitContext(LuaScriptingContext &context)
         "version",
         &osmium::Way::version);
 
-    struct RelationMemberWrap
+    context.state.new_usertype<osmium::RelationMember>("RelationMember",
-    {
+                                                       "role", &osmium::RelationMember::role,
-        explicit RelationMemberWrap(const osmium::RelationMember &member) { init(member); }
+                                                       "type", &osmium::RelationMember::type,
+                                                       "id", [](const osmium::RelationMember &member) -> osmium::object_id_type {
+                                                                 return member.ref();
+                                                             });
 
-        RelationMemberWrap() : item_type(osmium::item_type::undefined) {}
-
-        void init(const osmium::RelationMember &member)
-        {
-            role = member.role();
-            item_type = member.type();
-            id = member.ref();
-        }
-
-        const char *GetRole() const { return role.c_str(); }
-        osmium::item_type GetItemType() const { return item_type; }
-        osmium::object_id_type GetId() const { return id; }
-
-        util::OsmIDTyped ref() const
-        {
-            switch (item_type)
-            {
-            case osmium::item_type::node:
-                return util::OsmIDTyped(id, util::OsmIDTyped::Type::Node);
-
-            case osmium::item_type::way:
-                return util::OsmIDTyped(id, util::OsmIDTyped::Type::Way);
-
-            case osmium::item_type::relation:
-                return util::OsmIDTyped(id, util::OsmIDTyped::Type::Relation);
-
-            default:
-                break;
-            }
-
-            return util::OsmIDTyped(id, util::OsmIDTyped::Type::Unknown);
-        }
-
-        std::string role;
-        osmium::item_type item_type;
-        osmium::object_id_type id;
-    };
-
-    context.state.new_usertype<util::OsmIDTyped>(
-        "OsmIDTypes", "id", &util::OsmIDTyped::GetID, "type", &util::OsmIDTyped::GetType);
-
-    context.state.new_usertype<RelationMemberWrap>("RelationMember",
-                                                   "role",
-                                                   &RelationMemberWrap::GetRole,
-                                                   "item_type",
-                                                   &RelationMemberWrap::GetItemType,
-                                                   "id",
-                                                   &RelationMemberWrap::GetId);
-
-    /** TODO: make better solution with members iteration.
-     *  For this moment, just make vector of RelationMember wrappers
-     */
     context.state.new_usertype<osmium::Relation>("Relation",
                                                  "get_value_by_key",
                                                  &get_value_by_key<osmium::Relation>,
@@ -404,13 +355,8 @@ void Sol2ScriptingEnvironment::InitContext(LuaScriptingContext &context)
                                                  "version",
                                                  &osmium::Relation::version,
                                                  "members",
-                                                 [](const osmium::Relation &rel) {
+                                                 [](const osmium::Relation &rel) -> const osmium::RelationMemberList& {
-                                                     std::vector<RelationMemberWrap> members(
+                                                     return rel.members();
-                                                         rel.members().size());
-                                                     size_t i = 0;
-                                                     for (const auto &m : rel.members())
-                                                         members[i++].init(m);
-                                                     return sol::as_table(std::move(members));
                                                  });
 
     context.state.new_usertype<osmium::Node>("Node",
@@ -522,12 +468,12 @@ void Sol2ScriptingEnvironment::InitContext(LuaScriptingContext &context)
     context.state.new_usertype<ExtractionRelation>(
         "ExtractionRelation",
         sol::meta_function::new_index,
-        [](ExtractionRelation &rel, const RelationMemberWrap &member) -> ExtractionRelation::AttributesMap& {
+        [](ExtractionRelation &rel, const osmium::RelationMember &member) -> ExtractionRelation::AttributesMap& {
-            return rel.GetMember(member.ref());
+            return rel.GetMember(member);
         },
         sol::meta_function::index,
-        [](ExtractionRelation &rel, const RelationMemberWrap &member) -> ExtractionRelation::AttributesMap& {
+        [](ExtractionRelation &rel, const osmium::RelationMember &member) -> ExtractionRelation::AttributesMap& {
-            return rel.GetMember(member.ref());
+            return rel.GetMember(member);
         },
         "restriction",
         sol::property([](const ExtractionRelation &rel) { return rel.is_restriction; },
@@ -817,7 +763,7 @@ void Sol2ScriptingEnvironment::ProcessElements(
                  local_context.properties.call_tagless_node_function))
             {
                 const osmium::Node &node = static_cast<const osmium::Node &>(*entity);
-                const util::OsmIDTyped id(node.id(), util::OsmIDTyped::Type::Node);
+                const auto &id = ExtractionRelation::OsmIDTyped(node.id(), osmium::item_type::node);
                 local_context.ProcessNode(node, result_node, relations.Get(id));
             }
             resulting_nodes.push_back(std::pair<const osmium::Node &, ExtractionNode>(
@@ -828,7 +774,7 @@ void Sol2ScriptingEnvironment::ProcessElements(
             if (local_context.has_way_function)
             {
                 const osmium::Way &way = static_cast<const osmium::Way &>(*entity);
-                const util::OsmIDTyped id(way.id(), util::OsmIDTyped::Type::Way);
+                const auto &id = ExtractionRelation::OsmIDTyped(way.id(), osmium::item_type::way);
                 local_context.ProcessWay(way, result_way, relations.Get(id));
             }
             resulting_ways.push_back(std::pair<const osmium::Way &, ExtractionWay>(