[GitHub] [incubator-tvm] tqchen commented on a change in pull request #5740: [Object] Introduce POD-C Compliant tvm::Map

[GitBox <gi...@apache.org>]

tqchen commented on a change in pull request #5740:
URL: https://github.com/apache/incubator-tvm/pull/5740#discussion_r439890429



##########
File path: include/tvm/node/container.h
##########
@@ -43,51 +45,1184 @@ using runtime::Downcast;
 using runtime::IterAdapter;
 using runtime::make_object;
 using runtime::Object;
+using runtime::ObjectEqual;
+using runtime::ObjectHash;
 using runtime::ObjectPtr;
 using runtime::ObjectPtrEqual;
 using runtime::ObjectPtrHash;
 using runtime::ObjectRef;
 using runtime::String;
 using runtime::StringObj;
 
-/*! \brief String-aware ObjectRef hash functor */
-struct ObjectHash {
-  size_t operator()(const ObjectRef& a) const {
-    if (const auto* str = a.as<StringObj>()) {
-      return String::HashBytes(str->data, str->size);
-    }
-    return ObjectPtrHash()(a);
+#if (TVM_MAP_USE_STL != 0)
+
+/*! \brief Shared content of all specializations of hash map */
+class MapNode : public Object {
+ public:
+  /*! \brief Type of the keys in the hash map */
+  using key_type = ObjectRef;
+  /*! \brief Type of the values in the hash map */
+  using mapped_type = ObjectRef;
+  /*! \brief Type of the actual underlying container */
+  using ContainerType = std::unordered_map<ObjectRef, ObjectRef, ObjectHash, ObjectEqual>;
+  /*! \brief Iterator class */
+  using iterator = ContainerType::iterator;
+  /*! \brief Iterator class */
+  using const_iterator = ContainerType::const_iterator;
+  /*! \brief Type of value stored in the hash map */
+  using KVType = ContainerType::value_type;
+
+  static_assert(std::is_standard_layout<KVType>::value, "KVType is not standard layout");
+  static_assert(sizeof(KVType) == 16 || sizeof(KVType) == 8, "sizeof(KVType) incorrect");
+
+  static constexpr const uint32_t _type_index = runtime::TypeIndex::kRuntimeMap;
+  static constexpr const char* _type_key = "Map";
+  TVM_DECLARE_FINAL_OBJECT_INFO(MapNode, Object);
+
+  /*!
+   * \brief Number of elements in the SmallMapNode
+   * \return The result
+   */
+  size_t size() const { return data_.size(); }
+  /*!
+   * \brief Count the number of times a key exists in the hash map
+   * \param key The indexing key
+   * \return The result, 0 or 1
+   */
+  size_t count(const key_type& key) const { return data_.count(key); }
+  /*!
+   * \brief Index value associated with a key, throw exception if the key does not exist
+   * \param key The indexing key
+   * \return The const reference to the value
+   */
+  const mapped_type& at(const key_type& key) const { return data_.at(key); }
+  /*!
+   * \brief Index value associated with a key, throw exception if the key does not exist
+   * \param key The indexing key
+   * \return The mutable reference to the value
+   */
+  mapped_type& at(const key_type& key) { return data_.at(key); }
+  /*! \return begin iterator */
+  iterator begin() { return data_.begin(); }
+  /*! \return const begin iterator */
+  const_iterator begin() const { return data_.begin(); }
+  /*! \return end iterator */
+  iterator end() { return data_.end(); }
+  /*! \return end iterator */
+  const_iterator end() const { return data_.end(); }
+  /*!
+   * \brief Index value associated with a key
+   * \param key The indexing key
+   * \return The iterator of the entry associated with the key, end iterator if not exists
+   */
+  const_iterator find(const key_type& key) const { return data_.find(key); }
+  /*!
+   * \brief Index value associated with a key
+   * \param key The indexing key
+   * \return The iterator of the entry associated with the key, end iterator if not exists
+   */
+  iterator find(const key_type& key) { return data_.find(key); }
+  /*!
+   * \brief Erase the entry associated with the iterator
+   * \param position The iterator
+   */
+  void erase(const iterator& position) { data_.erase(position); }
+  /*!
+   * \brief Erase the entry associated with the key, do nothing if not exists
+   * \param key The indexing key
+   */
+  void erase(const key_type& key) { data_.erase(key); }
+
+ protected:
+  /*!
+   * \brief Create an empty container
+   * \return The object created
+   */
+  static ObjectPtr<MapNode> Empty() { return make_object<MapNode>(); }
+  /*!
+   * \brief Create the map using contents from the given iterators.
+   * \param first Begin of iterator
+   * \param last End of iterator
+   * \tparam IterType The type of iterator
+   * \return ObjectPtr to the map created
+   */
+  template <typename IterType>
+  static ObjectPtr<Object> CreateFromRange(IterType first, IterType last) {
+    ObjectPtr<MapNode> p = make_object<MapNode>();
+    p->data_ = std::unordered_map<ObjectRef, ObjectRef, ObjectHash, ObjectEqual>(first, last);
+    return p;
+  }
+  /*!
+   * \brief Insert an entry into the given hash map
+   * \param kv The entry to be inserted
+   * \param map The pointer to the map, can be changed if re-hashing happens
+   */
+  static void Insert(const KVType& kv, ObjectPtr<Object>* map) {

Review comment:
       Sounds good, my only reason to suggest Emplace was that we might be able to avoid copy, but given that Object copy cost is low, we should be fine here




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org