-
Notifications
You must be signed in to change notification settings - Fork 564
fix: Reduce idle CPU consumption of websocket server #1040
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
base: ros2
Are you sure you want to change the base?
Changes from 4 commits
f3e4b1f
093f730
25e9bf5
322e1d0
f3f5fbd
6d2eeba
2f50f11
f0ac970
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,21 +30,19 @@ | |
# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE | ||
# POSSIBILITY OF SUCH DAMAGE. | ||
|
||
import asyncio | ||
import sys | ||
import threading | ||
import traceback | ||
import uuid | ||
from collections import deque | ||
from functools import partial, wraps | ||
from functools import wraps | ||
|
||
from rosbridge_library.rosbridge_protocol import RosbridgeProtocol | ||
from rosbridge_library.util import bson | ||
from tornado.ioloop import IOLoop | ||
from tornado.iostream import StreamClosedError | ||
from tornado.websocket import WebSocketClosedError, WebSocketHandler | ||
|
||
_io_loop = IOLoop.instance() | ||
|
||
|
||
def _log_exception(): | ||
"""Log the most recent exception to ROS.""" | ||
|
@@ -124,6 +122,7 @@ class RosbridgeWebSocket(WebSocketHandler): | |
unregister_timeout = 10.0 # seconds | ||
bson_only_mode = False | ||
node_handle = None | ||
event_loop = None | ||
|
||
@log_exceptions | ||
def open(self): | ||
|
@@ -174,14 +173,16 @@ def on_close(self): | |
self.incoming_queue.finish() | ||
|
||
def send_message(self, message, compression="none"): | ||
cls = self.__class__ | ||
|
||
if isinstance(message, bson.BSON): | ||
binary = True | ||
elif compression in ["cbor", "cbor-raw"]: | ||
binary = True | ||
else: | ||
binary = False | ||
|
||
_io_loop.add_callback(partial(self.prewrite_message, message, binary)) | ||
asyncio.run_coroutine_threadsafe(self.prewrite_message(message, binary), cls.event_loop) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It probably will, the same with |
||
|
||
async def prewrite_message(self, message, binary): | ||
cls = self.__class__ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worried about this timeout being a hard-coded value, although it is just one spin...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need some timeout to make sure the thread wakes up once in a while to check for shutdown event. Alternatively, I can try running
executor.spin()
and callingexecutor.shutdown()
from the main thread.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried that and now one of the tests is timing out. I'm changing this PR to draft until I'll figure this out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests are timing out due to bugs in rclpy. I submitted a PR with the fix in ros2/rclpy#1469.