Skip to content

Commit

Permalink
Small performance improvments
Browse files Browse the repository at this point in the history
Address small performance issues to ROS2 impacting the `rmw_count_publishers` and `rmw_count_subscribers` method when called with a significante number of topics to count.
The root of the performance issues come from the duplication of `std::string` type of objects. These duplication are honerous with strings and cause a significante amount of CPU cycle to be wasted duplicating things.
  • Loading branch information
guillaumeautran committed Dec 29, 2017
1 parent 97a0998 commit 2349f98
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 13 deletions.
6 changes: 1 addition & 5 deletions rmw_fastrtps_cpp/src/demangle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@
std::string
_demangle_if_ros_topic(const std::string & topic_name)
{
std::string prefix = _get_ros_prefix_if_exists(topic_name);
if (prefix.length()) {
return topic_name.substr(strlen(ros_topic_prefix));
}
return topic_name;
return _strip_ros_prefix_if_exists(topic_name);
}

/// Return the demangled ROS type or the original if not a ROS type.
Expand Down
18 changes: 15 additions & 3 deletions rmw_fastrtps_cpp/src/namespace_prefix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,25 @@ const std::vector<std::string> _ros_prefixes =
} // extern "C"

/// Return the ROS specific prefix if it exists, otherwise "".
std::string
const std::string
_get_ros_prefix_if_exists(const std::string & topic_name)
{
for (auto prefix : _ros_prefixes) {
if (topic_name.rfind(std::string(prefix) + "/", 0) == 0) {
for (auto const &prefix : _ros_prefixes) {
if ((topic_name.rfind(prefix, 0) == 0) && (topic_name.at(prefix.length()) == '/')) {
return prefix;
}
}
return "";
}

/// Strip the ROS specific prefix if it exists from the topic name.
const std::string
_strip_ros_prefix_if_exists(const std::string & topic_name)
{
for (auto const &prefix : _ros_prefixes) {
if ((topic_name.rfind(prefix, 0) == 0) && (topic_name.at(prefix.length()) == '/')) {
return topic_name.substr(prefix.length());
}
}
return topic_name;
}
5 changes: 4 additions & 1 deletion rmw_fastrtps_cpp/src/namespace_prefix.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ extern const std::vector<std::string> _ros_prefixes;
} // extern "C"

/// Return the ROS specific prefix if it exists, otherwise "".
std::string
const std::string
_get_ros_prefix_if_exists(const std::string & topic_name);

/// Returns the topic name stripped of and ROS specific prefix if exists.
const std::string
_strip_ros_prefix_if_exists(const std::string & topic_name);
#endif // NAMESPACE_PREFIX_HPP_
8 changes: 4 additions & 4 deletions rmw_fastrtps_cpp/src/rmw_count.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ rmw_count_publishers(
WriterInfo * slave_target = impl->secondaryPubListener;
slave_target->mapmutex.lock();
*count = 0;
for (auto it : slave_target->topicNtypes) {
auto topic_fqdn = _demangle_if_ros_topic(it.first);
for (auto const &it : slave_target->topicNtypes) {
auto const topic_fqdn = _demangle_if_ros_topic(it.first);
if (topic_fqdn == topic_name) {
*count += it.second.size();
}
Expand Down Expand Up @@ -90,8 +90,8 @@ rmw_count_subscribers(
ReaderInfo * slave_target = impl->secondarySubListener;
*count = 0;
slave_target->mapmutex.lock();
for (auto it : slave_target->topicNtypes) {
auto topic_fqdn = _demangle_if_ros_topic(it.first);
for (auto const & it : slave_target->topicNtypes) {
auto const topic_fqdn = _demangle_if_ros_topic(it.first);
if (topic_fqdn == topic_name) {
*count += it.second.size();
}
Expand Down

0 comments on commit 2349f98

Please sign in to comment.