Skip to content

Fix 100% CPU usage when using libusb - #6083

Merged
dorodnic merged 10 commits into
realsenseai:developmentfrom
radfordi:fix-libusb-polling
Mar 25, 2020
Merged

Fix 100% CPU usage when using libusb#6083
dorodnic merged 10 commits into
realsenseai:developmentfrom
radfordi:fix-libusb-polling

Conversation

@radfordi

@radfordi radfordi commented Mar 18, 2020

Copy link
Copy Markdown
Contributor

Replace broken dispatcher with a thread to fix 100% CPU usage

The dispatcher can't tell the difference between "timeout" and "empty", so it ends up infinte looping. Using a thread directly is much simpler and more correct.

Fixes #5783. Fixes #6062.

@radfordi
radfordi changed the base branch from master to development March 18, 2020 01:39
@radfordi

Copy link
Copy Markdown
Contributor Author

@dorodnic, the broken dispatcher issue is not libusb or Mac specific. I see the issue with every program when the dispatcher's _queue is empty.

Comment thread src/libusb/enumerator-libusb.cpp Outdated
{
std::vector<usb_device_info> rv;
auto ctx = std::make_shared<usb_context>();
auto ctx = get_usb_context();

@radfordi radfordi Mar 18, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I went to clean up the fact that we are creating a usb_context every time we list devices, but before I could fix it, I noticed that @bfulkers-i had already fixed this as part of #5492!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I plan to rebase #5492 on top of this once this gets merged.

@radfordi
radfordi force-pushed the fix-libusb-polling branch 2 times, most recently from 4e7bf4a to 490e9a8 Compare March 19, 2020 03:01
@radfordi
radfordi force-pushed the fix-libusb-polling branch from c3ccbec to a6c17d2 Compare March 19, 2020 04:34
@dorodnic

Copy link
Copy Markdown
Contributor

Hi @radfordi
Thanks for the PR.
I'm trying to understand the fix, and am a bit confused.
This was consuming 100% CPU -

_event_handler = std::make_shared<active_object<>>([this](dispatcher::cancellable_timer cancellable_timer)	
            {	
                if(_kill_handler_thread)	
                    return;	
                auto sts = libusb_handle_events_completed(_ctx, &_kill_handler_thread);	
            });

However, this does not -

_event_handler = std::thread([this]() {
                    while (!_kill_handler_thread)
                        libusb_handle_events_completed(_ctx, &_kill_handler_thread);
                });

Why? Both are sniping a thread doing essentially the same thing...

It's unfortunate that active_object<> was not used correctly in the first place - the cancellable_timer object is supposed to provide uninterruptible sleep, not consuming CPU but also allowing the thread to be interrupted instantly.

I'll look deeper into it and merge the PR

@radfordi

radfordi commented Mar 23, 2020

Copy link
Copy Markdown
Contributor Author

Why? Both are sniping a thread doing essentially the same thing...

When _kill_handler_thread == 1, then in the new version the thread exits, while in the old version the handler just returns quickly. So what happens when there are no devices open? In the new version the thread doesn't exist, and in the old version the dispatcher loops calling the function as fast as it can with no waiting in between.

@dorodnic

Copy link
Copy Markdown
Contributor

Ok thank you
I'm testing the patch today and so far looks good to me 👍

@dorodnic
dorodnic merged commit f014199 into realsenseai:development Mar 25, 2020
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.

100% CPU Usage

2 participants