rmw_count_publisher|subscriber - excessive string allocation

[deng02]

Bug report

Required Info:

• Operating System:
  • Ubuntu 16.04
• Installation type:
  • source
• Version or commit hash:
  • HEAD
• DDS implementation:
  • Fast-RTPS
• Client library (if applicable):
  • N/A

Steps to reproduce issue

You can reproduce this in multiple ways but the easiest is to create a large number of ROS2 publishers and subscribers and run the dynamic bridge, which counts the ROS2 pub/subs every second.

Behavior

CPU utilization is higher than expected. Profiling shows a hotspot with operator new and cfree being called, which backtracks to the functions rmw_count_publishers and rmw_count_subscribers being called in the dnamic bridge. Specifically the pushback into the unfiltered_topics vector e.g. in rmw_count_subscribers:

std::map<std::string, std::vector<std::string>> unfiltered_topics;
  ReaderInfo * slave_target = impl->secondarySubListener;
  slave_target->mapmutex.lock();
  for (auto it : slave_target->topicNtypes) {
    for (auto & itt : it.second) {
      // truncate the ROS specific prefix
      auto topic_fqdn = _demangle_if_ros_topic(it.first);
      unfiltered_topics[topic_fqdn].push_back(itt);
    }
  }
  slave_target->mapmutex.unlock()

Similar code exists for the rmw_count_publishers function.

Code inspection shows that the vector values held by the unfiltered_topics map aren't used at all and that it's just the number we care about. Further, all of the topic_fqdn keys aren't really necessary since we're only really interested in one. The function only uses it for the purposes of a debug message.

Proposed fix

To reduce string allocation, drop the non-matching topic_fqdn keys and don't bother pushing the topic types onto the vector. Just count the number of topic types for matching topics and return.

[dirk-thomas]

Addressed by #167.

@deng02 If you use "Fixes #166" in the description of the pull request it will automatically close the referenced issue when the PR is being merged.

[deng02]

@dirk-thomas I'll definitely do that next time! Thanks for the review/merge.