diff --git a/net/dcsctp/tx/outstanding_data.cc b/net/dcsctp/tx/outstanding_data.cc index 91651e9a73..8e91ef9e3f 100644 --- a/net/dcsctp/tx/outstanding_data.cc +++ b/net/dcsctp/tx/outstanding_data.cc @@ -265,9 +265,10 @@ void OutstandingData::AbandonAllFor(const Item& item) { Data::IsEnd(true), item.data().is_unordered); Item& added_item = outstanding_data_ - .emplace(tsn, - Item(std::move(message_end), MaxRetransmits::NoLimit(), - TimeMs(0), TimeMs::InfiniteFuture())) + .emplace(std::piecewise_construct, std::forward_as_tuple(tsn), + std::forward_as_tuple(std::move(message_end), TimeMs(0), + MaxRetransmits::NoLimit(), + TimeMs::InfiniteFuture())) .first->second; // The added chunk shouldn't be included in `outstanding_bytes`, so set it // as acked. @@ -381,8 +382,8 @@ UnwrappedTSN OutstandingData::highest_outstanding_tsn() const { absl::optional OutstandingData::Insert( const Data& data, - MaxRetransmits max_retransmissions, TimeMs time_sent, + MaxRetransmits max_retransmissions, TimeMs expires_at) { UnwrappedTSN tsn = next_tsn_; next_tsn_.Increment(); @@ -392,8 +393,9 @@ absl::optional OutstandingData::Insert( outstanding_bytes_ += chunk_size; ++outstanding_items_; auto it = outstanding_data_ - .emplace(tsn, Item(data.Clone(), max_retransmissions, time_sent, - expires_at)) + .emplace(std::piecewise_construct, std::forward_as_tuple(tsn), + std::forward_as_tuple(data.Clone(), time_sent, + max_retransmissions, expires_at)) .first; if (it->second.has_expired(time_sent)) { diff --git a/net/dcsctp/tx/outstanding_data.h b/net/dcsctp/tx/outstanding_data.h index 5c638680b7..0332fdd38e 100644 --- a/net/dcsctp/tx/outstanding_data.h +++ b/net/dcsctp/tx/outstanding_data.h @@ -21,6 +21,7 @@ #include "net/dcsctp/packet/chunk/iforward_tsn_chunk.h" #include "net/dcsctp/packet/chunk/sack_chunk.h" #include "net/dcsctp/packet/data.h" +#include "net/dcsctp/public/types.h" namespace dcsctp { @@ -120,10 +121,11 @@ class OutstandingData { // Schedules `data` to be sent, with the provided partial reliability // parameters. Returns the TSN if the item was actually added and scheduled to // be sent, and absl::nullopt if it shouldn't be sent. - absl::optional Insert(const Data& data, - MaxRetransmits max_retransmissions, - TimeMs time_sent, - TimeMs expires_at); + absl::optional Insert( + const Data& data, + TimeMs time_sent, + MaxRetransmits max_retransmissions = MaxRetransmits::NoLimit(), + TimeMs expires_at = TimeMs::InfiniteFuture()); // Nacks all outstanding data. void NackAll(); @@ -162,15 +164,18 @@ class OutstandingData { kAbandon, }; - explicit Item(Data data, - MaxRetransmits max_retransmissions, - TimeMs time_sent, - TimeMs expires_at) - : max_retransmissions_(max_retransmissions), - time_sent_(time_sent), + Item(Data data, + TimeMs time_sent, + MaxRetransmits max_retransmissions, + TimeMs expires_at) + : time_sent_(time_sent), + max_retransmissions_(max_retransmissions), expires_at_(expires_at), data_(std::move(data)) {} + Item(const Item&) = delete; + Item& operator=(const Item&) = delete; + TimeMs time_sent() const { return time_sent_; } const Data& data() const { return data_; } @@ -208,7 +213,7 @@ class OutstandingData { bool has_expired(TimeMs now) const; private: - enum class Lifecycle { + enum class Lifecycle : uint8_t { // The chunk is alive (sent, received, etc) kActive, // The chunk is scheduled to be retransmitted, and will then transition to @@ -217,7 +222,7 @@ class OutstandingData { // The chunk has been abandoned. This is a terminal state. kAbandoned }; - enum class AckState { + enum class AckState : uint8_t { // The chunk is in-flight. kUnacked, // The chunk has been received and acknowledged. @@ -225,6 +230,19 @@ class OutstandingData { // The chunk has been nacked and is possibly lost. kNacked }; + + // NOTE: This data structure has been optimized for size, by ordering fields + // to avoid unnecessary padding. + + // When the packet was sent, and placed in this queue. + const TimeMs time_sent_; + // If the message was sent with a maximum number of retransmissions, this is + // set to that number. The value zero (0) means that it will never be + // retransmitted. + const MaxRetransmits max_retransmissions_; + // At this exact millisecond, the item is considered expired. If the message + // is not to be expired, this is set to the infinite future. + // Indicates the life cycle status of this chunk. Lifecycle lifecycle_ = Lifecycle::kActive; // Indicates the presence of this chunk, if it's in flight (Unacked), has @@ -236,17 +254,11 @@ class OutstandingData { uint8_t nack_count_ = 0; // The number of times the DATA chunk has been retransmitted. uint16_t num_retransmissions_ = 0; - // If the message was sent with a maximum number of retransmissions, this is - // set to that number. The value zero (0) means that it will never be - // retransmitted. - const MaxRetransmits max_retransmissions_; - // When the packet was sent, and placed in this queue. - const TimeMs time_sent_; - // At this exact millisecond, the item is considered expired. If the message - // is not to be expired, this is set to the infinite future. + const TimeMs expires_at_; + // The actual data to send/retransmit. - Data data_; + const Data data_; }; // Returns how large a chunk will be, serialized, carrying the data diff --git a/net/dcsctp/tx/outstanding_data_test.cc b/net/dcsctp/tx/outstanding_data_test.cc index 113f5f44e4..70f1f27eb0 100644 --- a/net/dcsctp/tx/outstanding_data_test.cc +++ b/net/dcsctp/tx/outstanding_data_test.cc @@ -64,10 +64,8 @@ TEST_F(OutstandingDataTest, HasInitialState) { } TEST_F(OutstandingDataTest, InsertChunk) { - ASSERT_HAS_VALUE_AND_ASSIGN( - UnwrappedTSN tsn, - buf_.Insert(gen_.Ordered({1}, "BE"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture())); + ASSERT_HAS_VALUE_AND_ASSIGN(UnwrappedTSN tsn, + buf_.Insert(gen_.Ordered({1}, "BE"), kNow)); EXPECT_EQ(tsn.Wrap(), TSN(10)); @@ -83,8 +81,7 @@ TEST_F(OutstandingDataTest, InsertChunk) { } TEST_F(OutstandingDataTest, AcksSingleChunk) { - buf_.Insert(gen_.Ordered({1}, "BE"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, "BE"), kNow); OutstandingData::AckInfo ack = buf_.HandleSack(unwrapper_.Unwrap(TSN(10)), {}, false); @@ -103,8 +100,7 @@ TEST_F(OutstandingDataTest, AcksSingleChunk) { } TEST_F(OutstandingDataTest, AcksPreviousChunkDoesntUpdate) { - buf_.Insert(gen_.Ordered({1}, "BE"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, "BE"), kNow); buf_.HandleSack(unwrapper_.Unwrap(TSN(9)), {}, false); EXPECT_EQ(buf_.outstanding_bytes(), DataChunk::kHeaderSize + RoundUpTo4(1)); @@ -119,10 +115,8 @@ TEST_F(OutstandingDataTest, AcksPreviousChunkDoesntUpdate) { } TEST_F(OutstandingDataTest, AcksAndNacksWithGapAckBlocks) { - buf_.Insert(gen_.Ordered({1}, "B"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, "E"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, "B"), kNow); + buf_.Insert(gen_.Ordered({1}, "E"), kNow); std::vector gab = {SackChunk::GapAckBlock(2, 2)}; OutstandingData::AckInfo ack = @@ -144,10 +138,8 @@ TEST_F(OutstandingDataTest, AcksAndNacksWithGapAckBlocks) { } TEST_F(OutstandingDataTest, NacksThreeTimesWithSameTsnDoesntRetransmit) { - buf_.Insert(gen_.Ordered({1}, "B"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, "E"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, "B"), kNow); + buf_.Insert(gen_.Ordered({1}, "E"), kNow); std::vector gab1 = {SackChunk::GapAckBlock(2, 2)}; EXPECT_FALSE( @@ -169,14 +161,10 @@ TEST_F(OutstandingDataTest, NacksThreeTimesWithSameTsnDoesntRetransmit) { } TEST_F(OutstandingDataTest, NacksThreeTimesResultsInRetransmission) { - buf_.Insert(gen_.Ordered({1}, "B"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, "E"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, "B"), kNow); + buf_.Insert(gen_.Ordered({1}, ""), kNow); + buf_.Insert(gen_.Ordered({1}, ""), kNow); + buf_.Insert(gen_.Ordered({1}, "E"), kNow); std::vector gab1 = {SackChunk::GapAckBlock(2, 2)}; EXPECT_FALSE( @@ -211,14 +199,10 @@ TEST_F(OutstandingDataTest, NacksThreeTimesResultsInRetransmission) { TEST_F(OutstandingDataTest, NacksThreeTimesResultsInAbandoning) { static constexpr MaxRetransmits kMaxRetransmissions(0); - buf_.Insert(gen_.Ordered({1}, "B"), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, "E"), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, "B"), kNow, kMaxRetransmissions); + buf_.Insert(gen_.Ordered({1}, ""), kNow, kMaxRetransmissions); + buf_.Insert(gen_.Ordered({1}, ""), kNow, kMaxRetransmissions); + buf_.Insert(gen_.Ordered({1}, "E"), kNow, kMaxRetransmissions); std::vector gab1 = {SackChunk::GapAckBlock(2, 2)}; EXPECT_FALSE( @@ -251,14 +235,10 @@ TEST_F(OutstandingDataTest, NacksThreeTimesResultsInAbandoning) { TEST_F(OutstandingDataTest, NacksThreeTimesResultsInAbandoningWithPlaceholder) { static constexpr MaxRetransmits kMaxRetransmissions(0); - buf_.Insert(gen_.Ordered({1}, "B"), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, "B"), kNow, kMaxRetransmissions); + buf_.Insert(gen_.Ordered({1}, ""), kNow, kMaxRetransmissions); + buf_.Insert(gen_.Ordered({1}, ""), kNow, kMaxRetransmissions); + buf_.Insert(gen_.Ordered({1}, ""), kNow, kMaxRetransmissions); std::vector gab1 = {SackChunk::GapAckBlock(2, 2)}; EXPECT_FALSE( @@ -292,17 +272,17 @@ TEST_F(OutstandingDataTest, NacksThreeTimesResultsInAbandoningWithPlaceholder) { TEST_F(OutstandingDataTest, ExpiresChunkBeforeItIsInserted) { static constexpr TimeMs kExpiresAt = kNow + DurationMs(1); - EXPECT_TRUE(buf_.Insert(gen_.Ordered({1}, "B"), MaxRetransmits::NoLimit(), - kNow, kExpiresAt) + EXPECT_TRUE(buf_.Insert(gen_.Ordered({1}, "B"), kNow, + MaxRetransmits::NoLimit(), kExpiresAt) .has_value()); - EXPECT_TRUE(buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), - kNow + DurationMs(0), kExpiresAt) + EXPECT_TRUE(buf_.Insert(gen_.Ordered({1}, ""), kNow + DurationMs(0), + MaxRetransmits::NoLimit(), kExpiresAt) .has_value()); EXPECT_CALL(on_discard_, Call(IsUnordered(false), StreamID(1), MID(42))) .WillOnce(Return(false)); - EXPECT_FALSE(buf_.Insert(gen_.Ordered({1}, "E"), MaxRetransmits::NoLimit(), - kNow + DurationMs(1), kExpiresAt) + EXPECT_FALSE(buf_.Insert(gen_.Ordered({1}, "E"), kNow + DurationMs(1), + MaxRetransmits::NoLimit(), kExpiresAt) .has_value()); EXPECT_FALSE(buf_.has_data_to_be_retransmitted()); @@ -318,12 +298,9 @@ TEST_F(OutstandingDataTest, ExpiresChunkBeforeItIsInserted) { TEST_F(OutstandingDataTest, CanGenerateForwardTsn) { static constexpr MaxRetransmits kMaxRetransmissions(0); - buf_.Insert(gen_.Ordered({1}, "B"), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, "E"), kMaxRetransmissions, kNow, - TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, "B"), kNow, kMaxRetransmissions); + buf_.Insert(gen_.Ordered({1}, ""), kNow, kMaxRetransmissions); + buf_.Insert(gen_.Ordered({1}, "E"), kNow, kMaxRetransmissions); EXPECT_CALL(on_discard_, Call(IsUnordered(false), StreamID(1), MID(42))) .WillOnce(Return(false)); @@ -342,22 +319,14 @@ TEST_F(OutstandingDataTest, CanGenerateForwardTsn) { } TEST_F(OutstandingDataTest, AckWithGapBlocksFromRFC4960Section334) { - buf_.Insert(gen_.Ordered({1}, "B"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, "E"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, "B"), kNow); + buf_.Insert(gen_.Ordered({1}, ""), kNow); + buf_.Insert(gen_.Ordered({1}, ""), kNow); + buf_.Insert(gen_.Ordered({1}, ""), kNow); + buf_.Insert(gen_.Ordered({1}, ""), kNow); + buf_.Insert(gen_.Ordered({1}, ""), kNow); + buf_.Insert(gen_.Ordered({1}, ""), kNow); + buf_.Insert(gen_.Ordered({1}, "E"), kNow); EXPECT_THAT(buf_.GetChunkStatesForTesting(), testing::ElementsAre(Pair(TSN(9), State::kAcked), // @@ -384,12 +353,9 @@ TEST_F(OutstandingDataTest, AckWithGapBlocksFromRFC4960Section334) { } TEST_F(OutstandingDataTest, MeasureRTT) { - buf_.Insert(gen_.Ordered({1}, "BE"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, "BE"), MaxRetransmits::NoLimit(), - kNow + DurationMs(1), TimeMs::InfiniteFuture()); - buf_.Insert(gen_.Ordered({1}, "BE"), MaxRetransmits::NoLimit(), - kNow + DurationMs(2), TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, "BE"), kNow); + buf_.Insert(gen_.Ordered({1}, "BE"), kNow + DurationMs(1)); + buf_.Insert(gen_.Ordered({1}, "BE"), kNow + DurationMs(2)); static constexpr DurationMs kDuration(123); ASSERT_HAS_VALUE_AND_ASSIGN( @@ -406,8 +372,10 @@ TEST_F(OutstandingDataTest, MustRetransmitBeforeGettingNackedAgain) { static constexpr MaxRetransmits kOneRetransmission(1); for (int tsn = 10; tsn <= 20; ++tsn) { - buf_.Insert(gen_.Ordered({1}, tsn == 10 ? "B" : tsn == 20 ? "E" : ""), - kOneRetransmission, kNow, TimeMs::InfiniteFuture()); + buf_.Insert(gen_.Ordered({1}, tsn == 10 ? "B" + : tsn == 20 ? "E" + : ""), + kNow, kOneRetransmission); } std::vector gab1 = {SackChunk::GapAckBlock(2, 2)}; @@ -484,16 +452,11 @@ TEST_F(OutstandingDataTest, CanAbandonChunksMarkedForFastRetransmit) { // 11 are NACKed three times, chunk 10 will be marked for retransmission, but // chunk 11 will be abandoned, which also abandons chunk 10, as it's part of // the same message. - buf_.Insert(gen_.Ordered({1}, "B"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); // 10 - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits(0), kNow, - TimeMs::InfiniteFuture()); // 11 - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); // 12 - buf_.Insert(gen_.Ordered({1}, ""), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); // 13 - buf_.Insert(gen_.Ordered({1}, "E"), MaxRetransmits::NoLimit(), kNow, - TimeMs::InfiniteFuture()); // 13 + buf_.Insert(gen_.Ordered({1}, "B"), kNow); // 10 + buf_.Insert(gen_.Ordered({1}, ""), kNow, MaxRetransmits(0)); // 11 + buf_.Insert(gen_.Ordered({1}, ""), kNow); // 12 + buf_.Insert(gen_.Ordered({1}, ""), kNow); // 13 + buf_.Insert(gen_.Ordered({1}, "E"), kNow); // 14 // ACK 9, 12 std::vector gab1 = {SackChunk::GapAckBlock(3, 3)}; diff --git a/net/dcsctp/tx/retransmission_queue.cc b/net/dcsctp/tx/retransmission_queue.cc index 0ca02b0b88..958879387e 100644 --- a/net/dcsctp/tx/retransmission_queue.cc +++ b/net/dcsctp/tx/retransmission_queue.cc @@ -464,10 +464,9 @@ std::vector> RetransmissionQueue::GetChunksToSend( rwnd_ -= chunk_size; absl::optional tsn = outstanding_data_.Insert( - chunk_opt->data, + chunk_opt->data, now, partial_reliability_ ? chunk_opt->max_retransmissions : MaxRetransmits::NoLimit(), - now, partial_reliability_ ? chunk_opt->expires_at : TimeMs::InfiniteFuture());