Skip to content

Feature/rec faster callbacks#2573

Draft
KerstinKeller wants to merge 4 commits intomasterfrom
feature/rec_faster_callbacks
Draft

Feature/rec faster callbacks#2573
KerstinKeller wants to merge 4 commits intomasterfrom
feature/rec_faster_callbacks

Conversation

@KerstinKeller
Copy link
Copy Markdown
Contributor

eCAL does not buffer any data. This is why it's extremly important for the eCAL recorder, to leave the data callbacks as quickly as possible.
At the moment, when recording, there might be as much as 3 locks that need to be locked when data arrives.
This might lead to congestion, if a lot of data is being sent simultaneously.
This PR addresses this issue by first forwarding the data to a concurrent queue, and doing all other processing from a different thread.

@KerstinKeller KerstinKeller added the cherry-pick-to-NONE Don't cherry-pick these changes label Mar 23, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread app/rec/rec_client_core/src/ecal_rec_impl.cpp
{
}

~ReceiveDispatchThread()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: annotate this function with 'override' or (rarely) 'final' [cppcoreguidelines-explicit-virtual-functions]

Suggested change
~ReceiveDispatchThread()
~ReceiveDispatchThread() override

{
}

~ReceiveDispatchThread()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: use '= default' to define a trivial destructor [modernize-use-equals-default]

app/rec/rec_client_core/src/ecal_rec_impl.cpp:80:

-       {
-       }
+       = default;

no_items = ecal_rec_impl.receive_dispatch_queue_.try_dequeue_bulk(token, frames.data(), frames.size());
for (size_t i = 0; i < no_items; ++i)
{
auto& frame = frames[i];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not use array subscript when the index is not an integer constant expression [cppcoreguidelines-pro-bounds-constant-array-index]

            auto& frame = frames[i];
                          ^

// Add to the pre-buffer (it is thread-safe by using a mutex internally)
for (size_t i = 0; i < no_items; ++i)
{
ecal_rec_impl.pre_buffer_.push_back(frames[i]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not use array subscript when the index is not an integer constant expression [cppcoreguidelines-pro-bounds-constant-array-index]

              ecal_rec_impl.pre_buffer_.push_back(frames[i]);
                                                  ^

Comment thread app/rec/rec_client_core/src/ecal_rec_impl.cpp Outdated
Comment thread app/rec/rec_client_core/src/ecal_rec_impl.cpp Outdated
Comment thread app/rec/rec_client_core/src/ecal_rec_impl.cpp
Comment thread app/rec/rec_client_core/src/ecal_rec_impl.cpp Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-pick-to-NONE Don't cherry-pick these changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant