search

ros / ros_comm

ROS communications-related packages, including core client libraries (roscpp, rospy, roslisp) and graph introspection tools (rostopic, rosnode, rosservice, rosparam).
http://wiki.ros.org/ros_comm
753 stars 911 forks source link

use itertools count for atomic increments #2280

Open graingert opened 2 years ago

peci1 commented 1 year ago

The provided code looks really suspicious. How is it guaranteed that the instance created by count() call is not garbage-collected? It would look more safe and descriptive to first store this instance and then use a lambda calling the iterator.

Apart from that, does this PR solve a particular performance issue, or does it just modernize the code?

For other reviewers: itertools.count() is implemented in C++, so it locks GIL instead of a mutex. This might be actually faster. Or might not...

graingert commented 1 year ago

This is the style used in the stdlib see https://github.com/python/cpython/blob/3.11/Lib/asyncio/tasks.py#L32

The count object can't be garbage collected because the bound method keeps a reference to it

peci1 commented 1 year ago

Thanks for the explanation. And regarding the performance part? Is there some benefit?