From f418f4870294d11cf02e77dcca7109aa81084c55 Mon Sep 17 00:00:00 2001 From: Daniel Collins Date: Mon, 11 Dec 2023 14:19:57 -0500 Subject: [PATCH] Change CallbackDeferrer to use a variant and callback pointer instead of std::function This should substantially reduce the overhead due to deferred callbacks in profiles. Bug: webrtc:15723 Change-Id: I4c52beb91eb08c9b0ac2d1ce9a4e11839aa35e38 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331020 Reviewed-by: Victor Boivie Commit-Queue: Daniel Collins Cr-Commit-Position: refs/heads/main@{#41363} --- net/dcsctp/socket/BUILD.gn | 1 + net/dcsctp/socket/callback_deferrer.cc | 111 +++++++++++++------------ net/dcsctp/socket/callback_deferrer.h | 17 +++- 3 files changed, 74 insertions(+), 55 deletions(-) diff --git a/net/dcsctp/socket/BUILD.gn b/net/dcsctp/socket/BUILD.gn index 04f61e5b72..9f40f76cf0 100644 --- a/net/dcsctp/socket/BUILD.gn +++ b/net/dcsctp/socket/BUILD.gn @@ -178,6 +178,7 @@ rtc_library("dcsctp_socket") { "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", + "//third_party/abseil-cpp/absl/types:variant", ] } diff --git a/net/dcsctp/socket/callback_deferrer.cc b/net/dcsctp/socket/callback_deferrer.cc index 0a24020167..e1079f7c96 100644 --- a/net/dcsctp/socket/callback_deferrer.cc +++ b/net/dcsctp/socket/callback_deferrer.cc @@ -12,31 +12,6 @@ #include "api/make_ref_counted.h" namespace dcsctp { -namespace { -// A wrapper around the move-only DcSctpMessage, to let it be captured in a -// lambda. -class MessageDeliverer { - public: - explicit MessageDeliverer(DcSctpMessage&& message) - : state_(rtc::make_ref_counted(std::move(message))) {} - - void Deliver(DcSctpSocketCallbacks& c) { - // Really ensure that it's only called once. - RTC_DCHECK(!state_->has_delivered); - state_->has_delivered = true; - c.OnMessageReceived(std::move(state_->message)); - } - - private: - struct State : public webrtc::RefCountInterface { - explicit State(DcSctpMessage&& m) - : has_delivered(false), message(std::move(m)) {} - bool has_delivered; - DcSctpMessage message; - }; - rtc::scoped_refptr state_; -}; -} // namespace void CallbackDeferrer::Prepare() { RTC_DCHECK(!prepared_); @@ -48,12 +23,12 @@ void CallbackDeferrer::TriggerDeferred() { // callback, and that might result in adding new callbacks to this instance, // and the vector can't be modified while iterated on. RTC_DCHECK(prepared_); - std::vector> deferred; + std::vector> deferred; deferred.swap(deferred_); prepared_ = false; - for (auto& cb : deferred) { - cb(underlying_); + for (auto& [cb, data] : deferred) { + cb(std::move(data), underlying_); } } @@ -84,40 +59,57 @@ uint32_t CallbackDeferrer::GetRandomInt(uint32_t low, uint32_t high) { void CallbackDeferrer::OnMessageReceived(DcSctpMessage message) { RTC_DCHECK(prepared_); deferred_.emplace_back( - [deliverer = MessageDeliverer(std::move(message))]( - DcSctpSocketCallbacks& cb) mutable { deliverer.Deliver(cb); }); + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + return cb.OnMessageReceived(absl::get(std::move(data))); + }, + std::move(message)); } void CallbackDeferrer::OnError(ErrorKind error, absl::string_view message) { RTC_DCHECK(prepared_); deferred_.emplace_back( - [error, message = std::string(message)](DcSctpSocketCallbacks& cb) { - cb.OnError(error, message); - }); + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + Error error = absl::get(std::move(data)); + return cb.OnError(error.error, error.message); + }, + Error{error, std::string(message)}); } void CallbackDeferrer::OnAborted(ErrorKind error, absl::string_view message) { RTC_DCHECK(prepared_); deferred_.emplace_back( - [error, message = std::string(message)](DcSctpSocketCallbacks& cb) { - cb.OnAborted(error, message); - }); + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + Error error = absl::get(std::move(data)); + return cb.OnAborted(error.error, error.message); + }, + Error{error, std::string(message)}); } void CallbackDeferrer::OnConnected() { RTC_DCHECK(prepared_); - deferred_.emplace_back([](DcSctpSocketCallbacks& cb) { cb.OnConnected(); }); + deferred_.emplace_back( + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + return cb.OnConnected(); + }, + absl::monostate{}); } void CallbackDeferrer::OnClosed() { RTC_DCHECK(prepared_); - deferred_.emplace_back([](DcSctpSocketCallbacks& cb) { cb.OnClosed(); }); + deferred_.emplace_back( + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + return cb.OnClosed(); + }, + absl::monostate{}); } void CallbackDeferrer::OnConnectionRestarted() { RTC_DCHECK(prepared_); deferred_.emplace_back( - [](DcSctpSocketCallbacks& cb) { cb.OnConnectionRestarted(); }); + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + return cb.OnConnectionRestarted(); + }, + absl::monostate{}); } void CallbackDeferrer::OnStreamsResetFailed( @@ -125,42 +117,53 @@ void CallbackDeferrer::OnStreamsResetFailed( absl::string_view reason) { RTC_DCHECK(prepared_); deferred_.emplace_back( - [streams = std::vector(outgoing_streams.begin(), - outgoing_streams.end()), - reason = std::string(reason)](DcSctpSocketCallbacks& cb) { - cb.OnStreamsResetFailed(streams, reason); - }); + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + StreamReset stream_reset = absl::get(std::move(data)); + return cb.OnStreamsResetFailed(stream_reset.streams, + stream_reset.message); + }, + StreamReset{{outgoing_streams.begin(), outgoing_streams.end()}, + std::string(reason)}); } void CallbackDeferrer::OnStreamsResetPerformed( rtc::ArrayView outgoing_streams) { RTC_DCHECK(prepared_); deferred_.emplace_back( - [streams = std::vector(outgoing_streams.begin(), - outgoing_streams.end())]( - DcSctpSocketCallbacks& cb) { cb.OnStreamsResetPerformed(streams); }); + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + StreamReset stream_reset = absl::get(std::move(data)); + return cb.OnStreamsResetPerformed(stream_reset.streams); + }, + StreamReset{{outgoing_streams.begin(), outgoing_streams.end()}}); } void CallbackDeferrer::OnIncomingStreamsReset( rtc::ArrayView incoming_streams) { RTC_DCHECK(prepared_); deferred_.emplace_back( - [streams = std::vector(incoming_streams.begin(), - incoming_streams.end())]( - DcSctpSocketCallbacks& cb) { cb.OnIncomingStreamsReset(streams); }); + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + StreamReset stream_reset = absl::get(std::move(data)); + return cb.OnIncomingStreamsReset(stream_reset.streams); + }, + StreamReset{{incoming_streams.begin(), incoming_streams.end()}}); } void CallbackDeferrer::OnBufferedAmountLow(StreamID stream_id) { RTC_DCHECK(prepared_); - deferred_.emplace_back([stream_id](DcSctpSocketCallbacks& cb) { - cb.OnBufferedAmountLow(stream_id); - }); + deferred_.emplace_back( + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + return cb.OnBufferedAmountLow(absl::get(std::move(data))); + }, + stream_id); } void CallbackDeferrer::OnTotalBufferedAmountLow() { RTC_DCHECK(prepared_); deferred_.emplace_back( - [](DcSctpSocketCallbacks& cb) { cb.OnTotalBufferedAmountLow(); }); + +[](CallbackData data, DcSctpSocketCallbacks& cb) { + return cb.OnTotalBufferedAmountLow(); + }, + absl::monostate{}); } void CallbackDeferrer::OnLifecycleMessageExpired(LifecycleId lifecycle_id, diff --git a/net/dcsctp/socket/callback_deferrer.h b/net/dcsctp/socket/callback_deferrer.h index 6659e87155..9d9fbcef06 100644 --- a/net/dcsctp/socket/callback_deferrer.h +++ b/net/dcsctp/socket/callback_deferrer.h @@ -18,6 +18,7 @@ #include #include "absl/strings/string_view.h" +#include "absl/types/variant.h" #include "api/array_view.h" #include "api/ref_counted_base.h" #include "api/scoped_refptr.h" @@ -89,12 +90,26 @@ class CallbackDeferrer : public DcSctpSocketCallbacks { void OnLifecycleEnd(LifecycleId lifecycle_id) override; private: + struct Error { + ErrorKind error; + std::string message; + }; + struct StreamReset { + std::vector streams; + std::string message; + }; + // Use a pre-sized variant for storage to avoid double heap allocation. This + // variant can hold all cases of stored data. + using CallbackData = absl:: + variant; + using Callback = void (*)(CallbackData, DcSctpSocketCallbacks&); + void Prepare(); void TriggerDeferred(); DcSctpSocketCallbacks& underlying_; bool prepared_ = false; - std::vector> deferred_; + std::vector> deferred_; }; } // namespace dcsctp