admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dcsctp: Make use of log_prefix consistent

Browse Source 
The log_prefix frequently used in dcSCTP is intended to be used
to separate logs from different sockets within the same log output,
typically in unit tests. Every log entry always has the file and
line, so it's not important to add more information to the log prefix
that indicates _where_ it's logged. So those have been removed.

Also, since log_prefix is a string (typically 32 bytes) and it's
never changing during the lifetime of the socket, pass and store it
as a absl::string_view to save memory.

Bug: None
Change-Id: I10466710ca6c2badfcd3adc5630426a90ca74204
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274704
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39571}
This commit is contained in:
Victor Boivie 2022-09-09 10:25:32 +00:00 committed by WebRTC LUCI CQ
parent 5da3eb0d89
commit 4fbf555989
17 changed files with 65 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
net/dcsctp/rx/data_tracker.h
View File

@ -55,7 +55,7 @@ class DataTracker {
DataTracker(absl::string_view log_prefix,
Timer* delayed_ack_timer,
TSN peer_initial_tsn)
: log_prefix_(std::string(log_prefix) + "dtrack: "),
: log_prefix_(log_prefix),
seen_packet_(false),
delayed_ack_timer_(*delayed_ack_timer),
last_cumulative_acked_tsn_(
@ -172,7 +172,7 @@ class DataTracker {
void UpdateAckState(AckState new_state, absl::string_view reason);
static absl::string_view ToString(AckState ack_state);
const std::string log_prefix_;
const absl::string_view log_prefix_;
// If a packet has ever been seen.
bool seen_packet_;
Timer& delayed_ack_timer_;

2
net/dcsctp/rx/interleaved_reassembly_streams.h
View File

@ -96,7 +96,7 @@ class InterleavedReassemblyStreams : public ReassemblyStreams {
Stream& GetOrCreateStream(const FullStreamId& stream_id);
const std::string log_prefix_;
const absl::string_view log_prefix_;
// Callback for when a message has been assembled.
const OnAssembledMessage on_assembled_message_;

2
net/dcsctp/rx/reassembly_queue.cc
View File

@ -53,7 +53,7 @@ ReassemblyQueue::ReassemblyQueue(absl::string_view log_prefix,
TSN peer_initial_tsn,
size_t max_size_bytes,
bool use_message_interleaving)
: log_prefix_(std::string(log_prefix) + "reasm: "),
: log_prefix_(log_prefix),
max_size_bytes_(max_size_bytes),
watermark_bytes_(max_size_bytes * kHighWatermarkLimit),
last_assembled_tsn_watermark_(

2
net/dcsctp/rx/reassembly_queue.h
View File

@ -138,7 +138,7 @@ class ReassemblyQueue {
std::vector<std::pair<TSN, Data>> deferred_chunks;
};
const std::string log_prefix_;
const absl::string_view log_prefix_;
const size_t max_size_bytes_;
const size_t watermark_bytes_;
UnwrappedTSN::Unwrapper tsn_unwrapper_;

2
net/dcsctp/rx/traditional_reassembly_streams.h
View File

@ -107,7 +107,7 @@ class TraditionalReassemblyStreams : public ReassemblyStreams {
UnwrappedSSN next_ssn_;
};
const std::string log_prefix_;
const absl::string_view log_prefix_;
// Callback for when a message has been assembled.
const OnAssembledMessage on_assembled_message_;

2
net/dcsctp/socket/heartbeat_handler.cc
View File

@ -88,7 +88,7 @@ HeartbeatHandler::HeartbeatHandler(absl::string_view log_prefix,
const DcSctpOptions& options,
Context* context,
TimerManager* timer_manager)
: log_prefix_(std::string(log_prefix) + "heartbeat: "),
: log_prefix_(log_prefix),
ctx_(context),
timer_manager_(timer_manager),
interval_duration_(options.heartbeat_interval),

2
net/dcsctp/socket/heartbeat_handler.h
View File

@ -53,7 +53,7 @@ class HeartbeatHandler {
absl::optional<DurationMs> OnIntervalTimerExpiry();
absl::optional<DurationMs> OnTimeoutTimerExpiry();
const std::string log_prefix_;
const absl::string_view log_prefix_;
Context* ctx_;
TimerManager* timer_manager_;
// The time for a connection to be idle before a heartbeat is sent.

4
net/dcsctp/socket/stream_reset_handler.h
View File

@ -72,7 +72,7 @@ class StreamResetHandler {
ReassemblyQueue* reassembly_queue,
RetransmissionQueue* retransmission_queue,
const DcSctpSocketHandoverState* handover_state = nullptr)
: log_prefix_(std::string(log_prefix) + "reset: "),
: log_prefix_(log_prefix),
ctx_(context),
data_tracker_(data_tracker),
reassembly_queue_(reassembly_queue),
@ -210,7 +210,7 @@ class StreamResetHandler {
// Expiration handler for the Reconfig timer.
absl::optional<DurationMs> OnReconfigTimerExpiry();
const std::string log_prefix_;
const absl::string_view log_prefix_;
Context* ctx_;
DataTracker* data_tracker_;
ReassemblyQueue* reassembly_queue_;

2
net/dcsctp/socket/transmission_control_block.h
View File

@ -152,7 +152,7 @@ class TransmissionControlBlock : public Context {
// Will be called when the delayed ack timer expires.
absl::optional<DurationMs> OnDelayedAckTimerExpiry();
const std::string log_prefix_;
const absl::string_view log_prefix_;
const DcSctpOptions options_;
TimerManager& timer_manager_;
// Negotiated capabilities that both peers support.

5
net/dcsctp/tx/retransmission_error_counter.h
View File

@ -27,8 +27,7 @@ class RetransmissionErrorCounter {
public:
RetransmissionErrorCounter(absl::string_view log_prefix,
const DcSctpOptions& options)
: log_prefix_(std::string(log_prefix) + "rtx-errors: "),
limit_(options.max_retransmissions) {}
: log_prefix_(log_prefix), limit_(options.max_retransmissions) {}
// Increments the retransmission timer. If the maximum error count has been
// reached, `false` will be returned.
@ -42,7 +41,7 @@ class RetransmissionErrorCounter {
int value() const { return counter_; }
private:
const std::string log_prefix_;
const absl::string_view log_prefix_;
const absl::optional<int> limit_;
int counter_ = 0;
};

2
net/dcsctp/tx/retransmission_queue.cc
View File

@ -65,7 +65,7 @@ RetransmissionQueue::RetransmissionQueue(
options_(options),
min_bytes_required_to_send_(options.mtu * kMinBytesRequiredToSendFactor),
partial_reliability_(supports_partial_reliability),
log_prefix_(std::string(log_prefix) + "tx: "),
log_prefix_(log_prefix),
data_chunk_header_size_(use_message_interleaving
? IDataChunk::kHeaderSize
: DataChunk::kHeaderSize),

2
net/dcsctp/tx/retransmission_queue.h
View File

@ -221,7 +221,7 @@ class RetransmissionQueue {
const size_t min_bytes_required_to_send_;
// If the peer supports RFC3758 - SCTP Partial Reliability Extension.
const bool partial_reliability_;
const std::string log_prefix_;
const absl::string_view log_prefix_;
// The size of the data chunk (DATA/I-DATA) header that is used.
const size_t data_chunk_header_size_;
// Called when a new RTT measurement has been done

4
net/dcsctp/tx/rr_send_queue.cc
View File

@ -36,11 +36,11 @@ RRSendQueue::RRSendQueue(absl::string_view log_prefix,
size_t mtu,
StreamPriority default_priority,
size_t total_buffered_amount_low_threshold)
: log_prefix_(std::string(log_prefix) + "fcfs: "),
: log_prefix_(log_prefix),
callbacks_(*callbacks),
buffer_size_(buffer_size),
default_priority_(default_priority),
scheduler_(mtu),
scheduler_(log_prefix_, mtu),
total_buffered_amount_(
[this]() { callbacks_.OnTotalBufferedAmountLow(); }) {
total_buffered_amount_.SetLowThreshold(total_buffered_amount_low_threshold);

2
net/dcsctp/tx/rr_send_queue.h
View File

@ -265,7 +265,7 @@ class RRSendQueue : public SendQueue {
TimeMs now,
size_t max_size);
const std::string log_prefix_;
const absl::string_view log_prefix_;
DcSctpSocketCallbacks& callbacks_;
const size_t buffer_size_;
const StreamPriority default_priority_;

38
net/dcsctp/tx/stream_scheduler.cc
View File

@ -39,13 +39,14 @@ absl::optional<SendQueue::DataToSend> StreamScheduler::Produce(
bool rescheduling =
enable_message_interleaving_ || !currently_sending_a_message_;
RTC_LOG(LS_VERBOSE) << "Producing data, rescheduling=" << rescheduling
<< ", active="
<< StrJoin(active_streams_, ", ",
[&](rtc::StringBuilder& sb, const auto& p) {
sb << *p->stream_id() << "@"
<< *p->next_finish_time();
});
RTC_DLOG(LS_VERBOSE) << log_prefix_
<< "Producing data, rescheduling=" << rescheduling
<< ", active="
<< StrJoin(active_streams_, ", ",
[&](rtc::StringBuilder& sb, const auto& p) {
sb << *p->stream_id() << "@"
<< *p->next_finish_time();
});
RTC_DCHECK(rescheduling || current_stream_ != nullptr);
@ -54,13 +55,13 @@ absl::optional<SendQueue::DataToSend> StreamScheduler::Produce(
if (rescheduling) {
auto it = active_streams_.begin();
current_stream_ = *it;
RTC_DLOG(LS_VERBOSE) << "Rescheduling to stream "
RTC_DLOG(LS_VERBOSE) << log_prefix_ << "Rescheduling to stream "
<< *current_stream_->stream_id();
active_streams_.erase(it);
current_stream_->ForceMarkInactive();
} else {
RTC_DLOG(LS_VERBOSE) << "Producing from previous stream: "
RTC_DLOG(LS_VERBOSE) << log_prefix_ << "Producing from previous stream: "
<< *current_stream_->stream_id();
RTC_DCHECK(absl::c_any_of(active_streams_, [this](const auto* p) {
return p == current_stream_;
@ -72,6 +73,7 @@ absl::optional<SendQueue::DataToSend> StreamScheduler::Produce(
if (!data.has_value()) {
RTC_DLOG(LS_VERBOSE)
<< log_prefix_
<< "There is no stream with data; Can't produce any data.";
RTC_DCHECK(IsConsistent());
@ -80,7 +82,7 @@ absl::optional<SendQueue::DataToSend> StreamScheduler::Produce(
RTC_DCHECK(data->data.stream_id == current_stream_->stream_id());
RTC_DLOG(LS_VERBOSE) << "Producing DATA, type="
RTC_DLOG(LS_VERBOSE) << log_prefix_ << "Producing DATA, type="
<< (data->data.is_unordered ? "unordered" : "ordered")
<< "::"
<< (*data->data.is_beginning && *data->data.is_end
@ -131,7 +133,8 @@ absl::optional<SendQueue::DataToSend> StreamScheduler::Stream::Produce(
if (data.has_value()) {
VirtualTime new_current = CalculateFinishTime(data->data.payload.size());
RTC_DLOG(LS_VERBOSE) << "Virtual time changed: " << *current_virtual_time_
RTC_DLOG(LS_VERBOSE) << parent_.log_prefix_
<< "Virtual time changed: " << *current_virtual_time_
<< " -> " << *new_current;
current_virtual_time_ = new_current;
}
@ -142,7 +145,7 @@ absl::optional<SendQueue::DataToSend> StreamScheduler::Stream::Produce(
bool StreamScheduler::IsConsistent() const {
for (Stream* stream : active_streams_) {
if (stream->next_finish_time_ == VirtualTime::Zero()) {
RTC_DLOG(LS_VERBOSE) << "Stream " << *stream->stream_id()
RTC_DLOG(LS_VERBOSE) << log_prefix_ << "Stream " << *stream->stream_id()
<< " is active, but has no next-finish-time";
return false;
}
@ -151,7 +154,8 @@ bool StreamScheduler::IsConsistent() const {
}
void StreamScheduler::Stream::MaybeMakeActive() {
RTC_DLOG(LS_VERBOSE) << "MaybeMakeActive(" << *stream_id() << ")";
RTC_DLOG(LS_VERBOSE) << parent_.log_prefix_ << "MaybeMakeActive("
<< *stream_id() << ")";
RTC_DCHECK(next_finish_time_ == VirtualTime::Zero());
size_t bytes_to_send_next = bytes_to_send_in_next_message();
if (bytes_to_send_next == 0) {
@ -167,8 +171,9 @@ void StreamScheduler::Stream::MakeActive(size_t bytes_to_send_next) {
VirtualTime next_finish_time = CalculateFinishTime(
std::min(bytes_to_send_next, parent_.max_payload_bytes_));
RTC_DCHECK_GT(*next_finish_time, 0);
RTC_DLOG(LS_VERBOSE) << "Making stream " << *stream_id()
<< " active, expiring at " << *next_finish_time;
RTC_DLOG(LS_VERBOSE) << parent_.log_prefix_ << "Making stream "
<< *stream_id() << " active, expiring at "
<< *next_finish_time;
RTC_DCHECK(next_finish_time_ == VirtualTime::Zero());
next_finish_time_ = next_finish_time;
RTC_DCHECK(!absl::c_any_of(parent_.active_streams_,
@ -177,7 +182,8 @@ void StreamScheduler::Stream::MakeActive(size_t bytes_to_send_next) {
}
void StreamScheduler::Stream::ForceMarkInactive() {
RTC_DLOG(LS_VERBOSE) << "Making stream " << *stream_id() << " inactive";
RTC_DLOG(LS_VERBOSE) << parent_.log_prefix_ << "Making stream "
<< *stream_id() << " inactive";
RTC_DCHECK(next_finish_time_ != VirtualTime::Zero());
next_finish_time_ = VirtualTime::Zero();
}

6
net/dcsctp/tx/stream_scheduler.h
View File

@ -157,8 +157,9 @@ class StreamScheduler {
// The `mtu` parameter represents the maximum SCTP packet size, which should
// be the same as `DcSctpOptions::mtu`.
explicit StreamScheduler(size_t mtu)
: max_payload_bytes_(mtu - SctpPacket::kHeaderSize -
StreamScheduler(absl::string_view log_prefix, size_t mtu)
: log_prefix_(log_prefix),
max_payload_bytes_(mtu - SctpPacket::kHeaderSize -
IDataChunk::kHeaderSize) {}
std::unique_ptr<Stream> CreateStream(StreamProducer* producer,
@ -198,6 +199,7 @@ class StreamScheduler {
bool IsConsistent() const;
const absl::string_view log_prefix_;
const size_t max_payload_bytes_;
// The current virtual time, as defined in the WFQ algorithm.

42
net/dcsctp/tx/stream_scheduler_test.cc
View File

@ -92,14 +92,14 @@ class TestStream {
// A scheduler without active streams doesn't produce data.
TEST(StreamSchedulerTest, HasNoActiveStreams) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
EXPECT_EQ(scheduler.Produce(TimeMs(0), kMtu), absl::nullopt);
}
// Stream properties can be set and retrieved
TEST(StreamSchedulerTest, CanSetAndGetStreamProperties) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
StrictMock<MockStreamProducer> producer;
auto stream =
@ -114,7 +114,7 @@ TEST(StreamSchedulerTest, CanSetAndGetStreamProperties) {
// A scheduler with a single stream produced packets from it.
TEST(StreamSchedulerTest, CanProduceFromSingleStream) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
StrictMock<MockStreamProducer> producer;
EXPECT_CALL(producer, Produce).WillOnce(CreateChunk(StreamID(1), MID(0)));
@ -131,7 +131,7 @@ TEST(StreamSchedulerTest, CanProduceFromSingleStream) {
// Switches between two streams after every packet.
TEST(StreamSchedulerTest, WillRoundRobinBetweenStreams) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
StrictMock<MockStreamProducer> producer1;
EXPECT_CALL(producer1, Produce)
@ -173,7 +173,7 @@ TEST(StreamSchedulerTest, WillRoundRobinBetweenStreams) {
// Switches between two streams after every packet, but keeps producing from the
// same stream when a packet contains of multiple fragments.
TEST(StreamSchedulerTest, WillRoundRobinOnlyWhenFinishedProducingChunk) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
StrictMock<MockStreamProducer> producer1;
EXPECT_CALL(producer1, Produce)
@ -235,7 +235,7 @@ TEST(StreamSchedulerTest, WillRoundRobinOnlyWhenFinishedProducingChunk) {
// Deactivates a stream before it has finished producing all packets.
TEST(StreamSchedulerTest, StreamsCanBeMadeInactive) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
StrictMock<MockStreamProducer> producer1;
EXPECT_CALL(producer1, Produce)
@ -259,7 +259,7 @@ TEST(StreamSchedulerTest, StreamsCanBeMadeInactive) {
// Resumes a paused stream - makes a stream active after inactivating it.
TEST(StreamSchedulerTest, SingleStreamCanBeResumed) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
StrictMock<MockStreamProducer> producer1;
// Callbacks are setup so that they hint that there is a MID(2) coming...
@ -289,7 +289,7 @@ TEST(StreamSchedulerTest, SingleStreamCanBeResumed) {
// Iterates between streams, where one is suddenly paused and later resumed.
TEST(StreamSchedulerTest, WillRoundRobinWithPausedStream) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
StrictMock<MockStreamProducer> producer1;
EXPECT_CALL(producer1, Produce)
@ -333,7 +333,7 @@ TEST(StreamSchedulerTest, WillRoundRobinWithPausedStream) {
// Verifies that packet counts are evenly distributed in round robin scheduling.
TEST(StreamSchedulerTest, WillDistributeRoundRobinPacketsEvenlyTwoStreams) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
TestStream stream1(scheduler, StreamID(1), StreamPriority(1));
TestStream stream2(scheduler, StreamID(2), StreamPriority(1));
@ -346,7 +346,7 @@ TEST(StreamSchedulerTest, WillDistributeRoundRobinPacketsEvenlyTwoStreams) {
// where a stream is suddenly made inactive, two are added, and then the paused
// stream is resumed.
TEST(StreamSchedulerTest, WillDistributeEvenlyWithPausedAndAddedStreams) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
TestStream stream1(scheduler, StreamID(1), StreamPriority(1));
TestStream stream2(scheduler, StreamID(2), StreamPriority(1));
@ -376,7 +376,7 @@ TEST(StreamSchedulerTest, WillDistributeEvenlyWithPausedAndAddedStreams) {
// Degrades to fair queuing with streams having identical priority.
TEST(StreamSchedulerTest, WillDoFairQueuingWithSamePriority) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
scheduler.EnableMessageInterleaving(true);
constexpr size_t kSmallPacket = 30;
@ -427,7 +427,7 @@ TEST(StreamSchedulerTest, WillDoFairQueuingWithSamePriority) {
// Will do weighted fair queuing with three streams having different priority.
TEST(StreamSchedulerTest, WillDoWeightedFairQueuingSameSizeDifferentPriority) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
scheduler.EnableMessageInterleaving(true);
StrictMock<MockStreamProducer> callback1;
@ -499,7 +499,7 @@ TEST(StreamSchedulerTest, WillDoWeightedFairQueuingSameSizeDifferentPriority) {
// Will do weighted fair queuing with three streams having different priority
// and sending different payload sizes.
TEST(StreamSchedulerTest, WillDoWeightedFairQueuingDifferentSizeAndPriority) {
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
scheduler.EnableMessageInterleaving(true);
constexpr size_t kSmallPacket = 20;
@ -584,7 +584,7 @@ TEST(StreamSchedulerTest, WillDistributeWFQPacketsInTwoStreamsByPriority) {
// A simple test with two streams of different priority, but sending packets
// of identical size. Verifies that the ratio of sent packets represent their
// priority.
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
scheduler.EnableMessageInterleaving(true);
TestStream stream1(scheduler, StreamID(1), StreamPriority(100), kPayloadSize);
@ -598,7 +598,7 @@ TEST(StreamSchedulerTest, WillDistributeWFQPacketsInTwoStreamsByPriority) {
TEST(StreamSchedulerTest, WillDistributeWFQPacketsInFourStreamsByPriority) {
// Same as `WillDistributeWFQPacketsInTwoStreamsByPriority` but with more
// streams.
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
scheduler.EnableMessageInterleaving(true);
TestStream stream1(scheduler, StreamID(1), StreamPriority(100), kPayloadSize);
@ -624,7 +624,7 @@ TEST(StreamSchedulerTest, WillDistributeFromTwoStreamsFairly) {
// as stream2, but with WFQ and a 4x priority increase, stream2 should 4x as
// many payload bytes on the wire. That translates to stream2 getting 8x as
// many packets on the wire as they are half as large.
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
// Enable WFQ scheduler.
scheduler.EnableMessageInterleaving(true);
@ -641,7 +641,7 @@ TEST(StreamSchedulerTest, WillDistributeFromTwoStreamsFairly) {
TEST(StreamSchedulerTest, WillDistributeFromFourStreamsFairly) {
// Same as `WillDistributeWeightedFairFromTwoStreamsFairly` but more
// complicated.
StreamScheduler scheduler(kMtu);
StreamScheduler scheduler("", kMtu);
// Enable WFQ scheduler.
scheduler.EnableMessageInterleaving(true);
@ -671,8 +671,8 @@ TEST(StreamSchedulerTest, WillDistributeFromFourStreamsFairly) {
// small for both streams. See `LargeMessageWithLargeMtu` for the same test, but
// with a larger MTU.
TEST(StreamSchedulerTest, SendLargeMessageWithSmallMtu) {
StreamScheduler scheduler(100 + SctpPacket::kHeaderSize +
IDataChunk::kHeaderSize);
StreamScheduler scheduler(
"", 100 + SctpPacket::kHeaderSize + IDataChunk::kHeaderSize);
scheduler.EnableMessageInterleaving(true);
StrictMock<MockStreamProducer> producer1;
@ -708,8 +708,8 @@ TEST(StreamSchedulerTest, SendLargeMessageWithSmallMtu) {
// Sending large messages with large MTU will not fragment messages and will
// send the message first from the stream that has the smallest message.
TEST(StreamSchedulerTest, SendLargeMessageWithLargeMtu) {
StreamScheduler scheduler(200 + SctpPacket::kHeaderSize +
IDataChunk::kHeaderSize);
StreamScheduler scheduler(
"", 200 + SctpPacket::kHeaderSize + IDataChunk::kHeaderSize);
scheduler.EnableMessageInterleaving(true);
StrictMock<MockStreamProducer> producer1;