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

camera_calibration: Fix excessive CPU usage due to queue synchronization #432

Merged
merged 2 commits into from
Jul 18, 2019

Conversation

valgur
Copy link
Contributor

@valgur valgur commented Jul 18, 2019

I was getting near 100% CPU usage with cameracalibrator.py even if no images (after then inital one) are being received and no actual processing is taking place.

This appeared to be caused by the queue synchronization code doing

while len(self.queue) == 0:
    time.sleep(0.1)

to wait for incoming images. This does not look too unreasonable, but replacing it with proper thread synchronization via locks fixed the excessive CPU usage.

Specifically, I replaced the deque with a slightly modified standard Queue that drops the oldest item instead of raising an exception when adding an item to an already full queue, just like deque. Queue also uses deque internally anyway, so in that sense the performance should be the same.

I also added a --queue-size parameter to allow adjustment of the max queue size. I found this useful when calibrating with images from a bag, where you want to use all available frames and set the playback rate as high as possible.

Relates to #112 and #121.

Fixes excessive (up to 100%) CPU usage caused by queue synchronization code.

The new, modified Queue class BufferQueue drops the oldest item when adding
to a full queue. This is identical to deque's behavior, but adds the
much more CPU-efficient thread synchronization mechanism of Queue.
Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I have no issues. Lets get a second approval and ship.

@JWhitleyWork
Copy link
Collaborator

SHIP IT

@JWhitleyWork JWhitleyWork merged commit e73a760 into ros-perception:melodic Jul 18, 2019
@SteveMacenski
Copy link
Member

(I do love some shipping graphics)

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

Successfully merging this pull request may close these issues.

3 participants