Use statically-typed views for better performance (#856)

Signed-off-by: Ashton Larkin <[email protected]> Signed-off-by: Louise Poubel <[email protected]> Signed-off-by: Jose Luis Rivero <[email protected]> Co-authored-by: Ian Chen <[email protected]> Co-authored-by: Louise Poubel <[email protected]> Co-authored-by: Jose Luis Rivero <[email protected]>
gazebosim · Aug 13, 2021 · eaf7aab · eaf7aab
1 parent e040ec2
commit eaf7aab
Show file tree

Hide file tree

Showing 18 changed files with 2,331 additions and 1,341 deletions.
diff --git a/Migration.md b/Migration.md
@@ -17,6 +17,33 @@ release will remove the deprecated code.
 * `dynamic_pose/info` topic is removed from `LogRecord` and `LogPlayback`
 since pose information is being logged in the `changed_state` topic.
 
+* The internal management of entities and components in the
+  `EntityComponentManager` has been updated to improve runtime performance. As a
+  result, several methods have been deprecated, and a few types have changed.
+  * **Deprecated**:
+    + All `EntityComponentManager` methods that use `ComponentKey` as an input
+      parameter.
+    + The `EntityComponentManager::First` method.
+    + The `ComponentId` and `ComponentKey` types are now deprecated. A
+      combination of `Entity` and `ComponentTypeId` should be used instead.
+    + The `components::StorageDescriptorBase` and
+      `components::StorageDescriptor<ComponentTypeT>` classes.
+    + Methods in `components::Factory` that have deprecated input parameter
+      types and/or deprecated return types.
+        - The version of `components::Factory::Register` which has a
+          `StorageDescriptorBase *` input parameter.
+        - `components::Factory::NewStorage`
+    + The `ComponentStorageBase` and `ComponentStorage<ComponentTypeT>` classes.
+  * **Modified**:
+    + `EntityComponentManager::CreateComponent` now returns a pointer to the
+      created component instead of a `ComponentKey`.
+    + `ComponentKey` has been modified to be a
+      `std::pair<ComponentTypeId, Entity>` (it used to be a
+      `std::pair<ComponentTypeId, ComponentId>`) since the `ComponentId` type
+      is now deprecated. `ComponentKey` has also been deprecated, so usage of
+      this type is discouraged (see the **Deprecated** section above for more
+      information about how to replace usage of `ComponentKey`).
+
 ## Ignition Gazebo 4.x to 5.x
 
 * Use `cli` component of `ignition-utils1`.

diff --git a/include/ignition/gazebo/EntityComponentManager.hh b/include/ignition/gazebo/EntityComponentManager.hh
@@ -129,7 +129,7 @@ namespace ignition
       /// \param[in] _entity The entity to check.
       /// \param[in] _key The component to check.
       /// \return True if the component key belongs to the entity.
-      public: bool EntityHasComponent(const Entity _entity,
+      public: bool IGN_DEPRECATED(6) EntityHasComponent(const Entity _entity,
                   const ComponentKey &_key) const;
 
       /// \brief Check whether an entity has a specific component type.
@@ -152,7 +152,7 @@ namespace ignition
       /// \param[in] _key A key that uniquely identifies a component.
       /// \return True if the entity and component existed and the component was
       ///  removed.
-      public: bool RemoveComponent(
+      public: bool IGN_DEPRECATED(6) RemoveComponent(
                   const Entity _entity, const ComponentKey &_key);
 
       /// \brief Remove a component from an entity based on a type id.
@@ -180,9 +180,12 @@ namespace ignition
       /// \param[in] _entity The entity that will be associated with
       /// the component.
       /// \param[in] _data Data used to construct the component.
-      /// \return Key that uniquely identifies the component.
+      /// \return A pointer to the component that was created. nullptr is
+      /// returned if the component was not able to be created. If _entity
+      /// does not exist, nullptr will be returned.
       public: template<typename ComponentTypeT>
-              ComponentKey CreateComponent(const Entity _entity,
+              ComponentTypeT *CreateComponent(
+                  const Entity _entity,
                   const ComponentTypeT &_data);
 
       /// \brief Get a component assigned to an entity based on a
@@ -206,14 +209,16 @@ namespace ignition
       /// \return The component associated with the key, or nullptr if the
       /// component could not be found.
       public: template<typename ComponentTypeT>
-              const ComponentTypeT *Component(const ComponentKey &_key) const;
+              const ComponentTypeT IGN_DEPRECATED(6) * Component(
+              const ComponentKey &_key) const;
 
       /// \brief Get a mutable component based on a key.
       /// \param[in] _key A key that uniquely identifies a component.
       /// \return The component associated with the key, or nullptr if the
       /// component could not be found.
       public: template<typename ComponentTypeT>
-              ComponentTypeT *Component(const ComponentKey &_key);
+              ComponentTypeT IGN_DEPRECATED(6) * Component(
+              const ComponentKey &_key);
 
       /// \brief Get a mutable component assigned to an entity based on a
       /// component type. If the component doesn't exist, create it and
@@ -222,7 +227,7 @@ namespace ignition
       /// \param[in] _default The value that should be used to construct
       /// the component in case the component doesn't exist.
       /// \return The component of the specified type assigned to the specified
-      /// entity.
+      /// entity. If _entity does not exist, nullptr is returned.
       public: template<typename ComponentTypeT>
               ComponentTypeT *ComponentDefault(Entity _entity,
               const typename ComponentTypeT::Type &_default =
@@ -261,20 +266,20 @@ namespace ignition
           Entity _entity) const;
 
       /// \brief The first component instance of the specified type.
-      /// \return First component instance of the specified type, or nullptr
-      /// if the type does not exist.
+      /// This function is now deprecated, and will always return nullptr.
+      /// \return nullptr.
       public: template<typename ComponentTypeT>
-              const ComponentTypeT *First() const;
+              const ComponentTypeT IGN_DEPRECATED(6) * First() const;
 
       /// \brief The first component instance of the specified type.
-      /// \return First component instance of the specified type, or nullptr
-      /// if the type does not exist.
+      /// This function is now deprecated, and will always return nullptr.
+      /// \return nullptr.
       public: template<typename ComponentTypeT>
-              ComponentTypeT *First();
+              ComponentTypeT IGN_DEPRECATED(6) * First();
 
       /// \brief Get an entity which matches the value of all the given
       /// components. For example, the following will return the entity which
-      /// has an name component equal to "name" and has a model component:
+      /// has a name component equal to "name" and has a model component:
       ///
       ///  auto entity = EntityByComponents(components::Name("name"),
       ///    components::Model());
@@ -523,6 +528,7 @@ namespace ignition
       /// and components.
       /// \detail The header of the message will not be populated, it is the
       /// responsibility of the caller to timestamp it before use.
+      /// \param[out] _state The serialized state message to populate.
       /// \param[in] _entities Entities to be serialized. Leave empty to get
       /// all entities.
       /// \param[in] _types Type ID of components to be serialized. Leave empty
@@ -611,24 +617,14 @@ namespace ignition
       /// \return True if the Entity has been marked to be removed.
       private: bool IsMarkedForRemoval(const Entity _entity) const;
 
-      /// \brief Delete an existing Entity.
-      /// \param[in] _entity The entity to remove.
-      /// \returns True if the Entity existed and was deleted.
-      private: bool RemoveEntity(const Entity _entity);
-
-      /// \brief The first component instance of the specified type.
-      /// \return First component instance of the specified type, or nullptr
-      /// if the type does not exist.
-      private: components::BaseComponent *First(
-                   const ComponentTypeId _componentTypeId);
-
       /// \brief Implementation of CreateComponent.
       /// \param[in] _entity The entity that will be associated with
       /// the component.
       /// \param[in] _componentTypeId Id of the component type.
       /// \param[in] _data Data used to construct the component.
-      /// \return Key that uniquely identifies the component.
-      private: ComponentKey CreateComponentImplementation(
+      /// \return True if the component's data needs to be set externally; false
+      /// otherwise.
+      private: bool CreateComponentImplementation(
                    const Entity _entity,
                    const ComponentTypeId _componentTypeId,
                    const components::BaseComponent *_data);
@@ -651,79 +647,29 @@ namespace ignition
                    const Entity _entity,
                    const ComponentTypeId _type);
 
-      /// \brief Get a component based on a key.
-      /// \param[in] _key A key that uniquely identifies a component.
-      /// \return The component associated with the key, or nullptr if the
-      /// component could not be found.
-      private: const components::BaseComponent *ComponentImplementation(
-                   const ComponentKey &_key) const;
-
-      /// \brief Get a mutable component based on a key.
-      /// \param[in] _key A key that uniquely identifies a component.
-      /// \return The component associated with the key, or nullptr if the
-      /// component could not be found.
-      private: components::BaseComponent *ComponentImplementation(
-                   const ComponentKey &_key);
-
-      /// \brief End of the AddComponentToView recursion. This function is
-      /// called when Rest is empty.
-      /// \param[in, out] _view The FirstComponent will be added to the
-      /// _view.
-      /// \param[in] _entity The entity.
-      private: template<typename FirstComponent,
-                        typename ...RemainingComponents,
-                        typename std::enable_if<
-                          sizeof...(RemainingComponents) == 0, int>::type = 0>
-          void AddComponentsToView(detail::View &_view,
-               const Entity _entity) const;
-
-      /// \brief Recursively add components to a view. This function is
-      /// called when Rest is NOT empty.
-      /// \param[in, out] _view The FirstComponent will be added to the
-      /// _view.
-      /// \param[in] _entity The entity.
-      private: template<typename FirstComponent,
-                        typename ...RemainingComponents,
-                        typename std::enable_if<
-                          sizeof...(RemainingComponents) != 0, int>::type = 0>
-          void AddComponentsToView(detail::View &_view,
-              const Entity _entity) const;
-
       /// \brief Find a View that matches the set of ComponentTypeIds. If
       /// a match is not found, then a new view is created.
       /// \tparam ComponentTypeTs All the component types that define a view.
-      /// \return A reference to the view.
+      /// \return A pointer to the view.
       private: template<typename ...ComponentTypeTs>
-          detail::View &FindView() const;
+          detail::View<ComponentTypeTs...> *FindView() const;
 
       /// \brief Find a view based on the provided component type ids.
       /// \param[in] _types The component type ids that serve as a key into
       /// a map of views.
-      /// \param[out] _iter Iterator to the found element in the view map.
-      /// Check the return value to see if this iterator is valid.
-      /// \return True if the view was found, false otherwise.
-      private: bool FindView(const std::set<ComponentTypeId> &_types,
-          std::map<detail::ComponentTypeKey,
-          detail::View>::iterator &_iter) const;  // NOLINT
+      /// \return A pointer to the view. nullptr is returned if the view wasn't
+      /// found.
+      private: detail::BaseView *FindView(
+                   const std::vector<ComponentTypeId> &_types) const;
 
       /// \brief Add a new view to the set of stored views.
-      /// \param[in] _types The set of component type ids that is the key
+      /// \param[in] _types The set of component type ids that act as the key
       /// for the view.
       /// \param[in] _view The view to add.
-      /// \return An iterator to the view.
-      private: std::map<detail::ComponentTypeKey, detail::View>::iterator
-          AddView(const std::set<ComponentTypeId> &_types,
-              detail::View &&_view) const;
-
-      /// \brief Update views that contain the provided entity.
-      /// \param[in] _entity The entity.
-      private: void UpdateViews(const Entity _entity);
-
-      /// \brief Get a component ID based on an entity and the component's type.
-      /// \param[in] _entity The entity.
-      /// \param[in] _type Component type ID.
-      private: ComponentId EntityComponentIdFromType(
-          const Entity _entity, const ComponentTypeId _type) const;
+      /// \return A pointer to the view.
+      private: detail::BaseView *AddView(
+                   const detail::ComponentTypeKey &_types,
+                   std::unique_ptr<detail::BaseView> _view) const;
 
       /// \brief Add an entity and its components to a serialized state message.
       /// \param[out] _msg The state message.
@@ -761,10 +707,6 @@ namespace ignition
       // states. Like the runners, the managers are internal.
       friend class NetworkManagerPrimary;
       friend class NetworkManagerSecondary;
-
-      // Make View a friend so that it can access components.
-      // This should be safe since View is internal to Gazebo.
-      friend class detail::View;
     };
     }
   }

diff --git a/include/ignition/gazebo/Types.hh b/include/ignition/gazebo/Types.hh
@@ -22,6 +22,8 @@
 #include <functional>
 #include <utility>
 
+#include "ignition/gazebo/Entity.hh"
+
 namespace ignition
 {
   namespace gazebo
@@ -78,6 +80,8 @@ namespace ignition
     /// \brief A unique identifier for a component instance. The uniqueness
     /// of a ComponentId is scoped to the component's type.
     /// \sa ComponentKey.
+    /// \deprecated Deprecated on version 6, removed on version 7. Use
+    /// ComponentTypeId + Entity instead.
     using ComponentId = int;
 
     /// \brief A unique identifier for a component type. A component type
@@ -87,7 +91,10 @@ namespace ignition
 
     /// \brief A key that uniquely identifies, at the global scope, a component
     /// instance
-    using ComponentKey = std::pair<ComponentTypeId, ComponentId>;
+    /// \note On version 6, the 2nd element was changed to the entity ID.
+    /// \deprecated Deprecated on version 6, removed on version 7. Use
+    /// ComponentTypeId + Entity instead.
+    using ComponentKey = std::pair<ComponentTypeId, Entity>;
 
     /// \brief typedef for query callbacks
     using EntityQueryCallback = std::function<void (const UpdateInfo,