dds stream threading & participant send/receive buffer size #11560
Conversation
maloel
commented
Mar 13, 2023
maloel
commented
- change participant send/receive buffer size for improved performance, otherwise we have dropped frames
- stream and metadata callbacks are now in own threads
| @@ -33,6 +33,7 @@ class dds_subscriber; | |||
| // | |||
| class dds_topic_reader : public eprosima::fastdds::dds::DataReaderListener | |||
There was a problem hiding this comment.
Destructor should be virtual
| @@ -51,7 +51,7 @@ class dds_device::impl | |||
| std::queue< nlohmann::json > _option_response_queue; | |||
|
|
|||
| std::shared_ptr< dds_topic_reader > _notifications_reader; | |||
There was a problem hiding this comment.
Why are you not handling notifications in a thread?
There are not a lot of traffic but we parse json and do some other jobs that can take time
There was a problem hiding this comment.
I can change, though it may have side-effects -- I'd rather do separately.
There was a problem hiding this comment.
Actually, let me try
There was a problem hiding this comment.
The notifications reader has mixed usage, split amongst init() and regular notifications, with the former actually calling wait_for_unread_message. I.e., this complicates things. For now, getting notifications on the eProsima thread while all the rest are using custom threads should be OK.
I think we should push this to another PR.
There was a problem hiding this comment.
OK. We should monitor CPU usage and option setting delay with high resolution streaming and fix if needed.
| @@ -72,7 +73,7 @@ class dds_topic_reader : public eprosima::fastdds::dds::DataReaderListener | |||
| }; | |||
|
|
|||
| // The callbacks should be set before we actually create the underlying DDS objects, so the reader does not | |||
| void run( qos const & = qos() ); | |||
| virtual void run( qos const & = qos() ); | |||
There was a problem hiding this comment.
I think it is better to remove the default parameter. Having default for virtual functions can be error prone because the base default will always be used even if a son changes the default.
We don't use the default in our code now, we pass a parameter everywhere used, and future users can just create the qos object when calling run.
There was a problem hiding this comment.
the base default will always be used
Only if you point to the base.
We don't use the default in our code now
We do... in the watcher, flexible-reader. I'll change them, too...
| std::shared_ptr< dds_subscriber > const & subscriber ); | ||
|
|
||
| // The callbacks should be set before we actually create the underlying DDS objects, so the reader does not | ||
| void run( qos const & = qos() ); |
There was a problem hiding this comment.
Better to mark this as override
