Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

__debug__ flag slows down image message creation #79

Closed
mlix11 opened this issue Aug 12, 2019 · 2 comments
Closed

__debug__ flag slows down image message creation #79

mlix11 opened this issue Aug 12, 2019 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@mlix11
Copy link

mlix11 commented Aug 12, 2019

Bug report

Required Info:

  • Operating System:
    • Ubuntu 18.04
  • Installation type:
    • Debian package
  • Version or commit hash:
    • dashing
  • DDS implementation:
    • default
  • Client library (if applicable):
    • rclpy

Steps to reproduce issue

from time import time
import numpy as np
from cv_bridge import CvBridge

# initialize cv_bridge
bridge = CvBridge()
# generate fake image with numpy
fake_image = np.random.randint(254, size=(1080, 1920, 3), dtype=np.uint8)
print(f"Fake image size: {fake_image.nbytes / 1048576}MB")

for i in range(10):
    n = time()
    bridge.cv2_to_imgmsg(fake_image)
    print(f"{time()-n}s")

Expected behavior

Output:
Fake image size: 5.9326171875MB
0.04487729072570801s
0.007574319839477539s
0.0013623237609863281s
0.001272439956665039s
0.0012216567993164062s
0.0011925697326660156s
0.0012187957763671875s
0.0012259483337402344s
0.0011894702911376953s
0.0010957717895507812s

Actual behavior

Output:
Fake image size: 5.9326171875MB
1.4624004364013672s
1.4479649066925049s
1.45872163772583s
1.4582252502441406s
1.4557397365570068s
1.4485929012298584s
1.4805355072021484s
1.5637807846069336s
1.5311927795410156s
1.6195440292358398s

Additional information

I experienced a slow performance during a cv2_to_imgmsg call. The interesting part is, that the call in C++ is fast. Probably it has to do something with the python message wrapper. Did someone experienced also a slow down with cv2_to_imgmsg calls?
Right now a 1080x1920x3 image takes up 1.4s to convert to a ROS2 image message.

The slow down is happening at this line: https://github.com/ros-perception/vision_opencv/blob/ros2/cv_bridge/python/cv_bridge/core.py#L275
and the root cause is at this line: https://github.com/ros2/rosidl_python/blob/master/rosidl_generator_py/resource/_msg.py.em#L481

Feature request

Deactivate the per-element comparison for images or use another approach then the
__debug__ flag from python. This flag is always on and can be only set to false with PYTHONOPTIMIZE={1|2} (for ROS2) or for python (-O) (-OO). Did not found a documentation for this behavior in the ROS2 documentation. With this flag the __debug__ field is going to be false and everything is running smoothly.

@dirk-thomas
Copy link
Member

The check of the data types of each element in the sequence is relevant to prevent putting invalid data into the message.

As of Dashing a uint8[] is represented by an array.array with the typecode B. If the value being assigned is of that type (which already intrinsically ensures the right data type of each value the check doesn't need to be performed (see

if isinstance(value, array.array):
assert value.typecode == '@(SPECIAL_NESTED_BASIC_TYPES[member.type.value_type.typename]['type_code'])', \
"The '@(member.name)' array.array() must have the type code of '@(SPECIAL_NESTED_BASIC_TYPES[member.type.value_type.typename]['type_code'])'"
@[ if isinstance(member.type, BoundedSequence)]@
assert len(value) <= @(member.type.maximum_size), \
"The '@(member.name)' array.array() must have a size <= @(member.type.maximum_size)"
@[ end if]@
self._@(member.name) = value
return
).

Therefore I think the function cv2_to_imgmsg in cv_bridge should be modified to not convert the data to a string and assign that. Instead it should populate the array.array directly from numpy.ndarray.

See ros-perception/vision_opencv#305 for the proposed fix which updates cv2_to_imgmsg as well as cv2_to_compressed_imgmsg.

@dirk-thomas
Copy link
Member

Addressed by the referenced PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants