gfe-relnote: deprecate gfe2_reloadable_flag_quic_interval_deque PiperOrigin-RevId: 295842077 Change-Id: Ieeab3c1c815fdae7e02e6a2c878a35d5cd497e0d
diff --git a/quic/core/quic_stream_send_buffer.cc b/quic/core/quic_stream_send_buffer.cc index c7112ff..8c31be8 100644 --- a/quic/core/quic_stream_send_buffer.cc +++ b/quic/core/quic_stream_send_buffer.cc
@@ -45,9 +45,7 @@ } QuicStreamSendBuffer::QuicStreamSendBuffer(QuicBufferAllocator* allocator) - // TODO(b/144690240): Remove this variable once quic_seeker is deprecated. - : interval_deque_active_(GetQuicReloadableFlag(quic_interval_deque)), - current_end_offset_(0), + : current_end_offset_(0), stream_offset_(0), allocator_(allocator), stream_bytes_written_(0), @@ -83,21 +81,13 @@ return; } size_t length = slice.length(); - if (interval_deque_active_) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_interval_deque, 1, 5); - // Need to start the offsets at the right interval. - if (interval_deque_.Empty()) { - const QuicStreamOffset end = stream_offset_ + length; - current_end_offset_ = std::max(current_end_offset_, end); - } - BufferedSlice bs = BufferedSlice(std::move(slice), stream_offset_); - interval_deque_.PushBack(std::move(bs)); - } else { - buffered_slices_.emplace_back(std::move(slice), stream_offset_); - if (write_index_ == -1) { - write_index_ = buffered_slices_.size() - 1; - } + // Need to start the offsets at the right interval. + if (interval_deque_.Empty()) { + const QuicStreamOffset end = stream_offset_ + length; + current_end_offset_ = std::max(current_end_offset_, end); } + BufferedSlice bs = BufferedSlice(std::move(slice), stream_offset_); + interval_deque_.PushBack(std::move(bs)); stream_offset_ += length; } @@ -114,63 +104,18 @@ bool QuicStreamSendBuffer::WriteStreamData(QuicStreamOffset offset, QuicByteCount data_length, QuicDataWriter* writer) { - if (interval_deque_active_) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_interval_deque, 2, 5); - QUIC_BUG_IF(current_end_offset_ < offset) - << "Tried to write data out of sequence. last_offset_end:" - << current_end_offset_ << ", offset:" << offset; - // The iterator returned from |interval_deque_| will automatically advance - // the internal write index for the QuicIntervalDeque. The incrementing is - // done in operator++. - for (auto slice_it = interval_deque_.DataAt(offset); - slice_it != interval_deque_.DataEnd(); ++slice_it) { - if (data_length == 0 || offset < slice_it->offset) { - break; - } - - QuicByteCount slice_offset = offset - slice_it->offset; - QuicByteCount available_bytes_in_slice = - slice_it->slice.length() - slice_offset; - QuicByteCount copy_length = - std::min(data_length, available_bytes_in_slice); - if (!writer->WriteBytes(slice_it->slice.data() + slice_offset, - copy_length)) { - QUIC_BUG << "Writer fails to write."; - return false; - } - offset += copy_length; - data_length -= copy_length; - const QuicStreamOffset new_end = - slice_it->offset + slice_it->slice.length(); - current_end_offset_ = std::max(current_end_offset_, new_end); - } - return data_length == 0; - } - - QuicCircularDeque<BufferedSlice>::iterator slice_it = - write_index_ == -1 - ? buffered_slices_.begin() - // Assume with write_index, write mostly starts from indexed slice. - : buffered_slices_.begin() + write_index_; - if (write_index_ != -1) { - if (offset >= slice_it->offset + slice_it->slice.length()) { - QUIC_BUG << "Tried to write data out of sequence."; - return false; - } - // Determine if write actually happens at indexed slice. - if (offset < slice_it->offset) { - // Write index missed, move iterator to the beginning. - slice_it = buffered_slices_.begin(); - } - } - - for (; slice_it != buffered_slices_.end(); ++slice_it) { + QUIC_BUG_IF(current_end_offset_ < offset) + << "Tried to write data out of sequence. last_offset_end:" + << current_end_offset_ << ", offset:" << offset; + // The iterator returned from |interval_deque_| will automatically advance + // the internal write index for the QuicIntervalDeque. The incrementing is + // done in operator++. + for (auto slice_it = interval_deque_.DataAt(offset); + slice_it != interval_deque_.DataEnd(); ++slice_it) { if (data_length == 0 || offset < slice_it->offset) { break; } - if (offset >= slice_it->offset + slice_it->slice.length()) { - continue; - } + QuicByteCount slice_offset = offset - slice_it->offset; QuicByteCount available_bytes_in_slice = slice_it->slice.length() - slice_offset; @@ -182,24 +127,10 @@ } offset += copy_length; data_length -= copy_length; - - if (write_index_ != -1) { - QuicCircularDeque<BufferedSlice>::const_iterator index_slice = - buffered_slices_.begin() + write_index_; - if (index_slice->offset == slice_it->offset && - copy_length == available_bytes_in_slice) { - // The slice pointed by write_index has been fully written, advance - // write index. - ++write_index_; - } - } + const QuicStreamOffset new_end = + slice_it->offset + slice_it->slice.length(); + current_end_offset_ = std::max(current_end_offset_, new_end); } - - if (write_index_ != -1 && - static_cast<size_t>(write_index_) == buffered_slices_.size()) { - write_index_ = -1; - } - return data_length == 0; } @@ -295,60 +226,26 @@ bool QuicStreamSendBuffer::FreeMemSlices(QuicStreamOffset start, QuicStreamOffset end) { - if (interval_deque_active_) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_interval_deque, 3, 5); - auto it = interval_deque_.DataBegin(); - if (it == interval_deque_.DataEnd() || it->slice.empty()) { - QUIC_BUG << "Trying to ack stream data [" << start << ", " << end << "), " - << (it == interval_deque_.DataEnd() - ? "and there is no outstanding data." - : "and the first slice is empty."); - return false; - } - if (!it->interval().Contains(start)) { - // Slow path that not the earliest outstanding data gets acked. - it = std::lower_bound(interval_deque_.DataBegin(), - interval_deque_.DataEnd(), start, CompareOffset()); - } - if (it == interval_deque_.DataEnd() || it->slice.empty()) { - QUIC_BUG << "Offset " << start << " with iterator offset: " << it->offset - << (it == interval_deque_.DataEnd() - ? " does not exist." - : " has already been acked."); - return false; - } - for (; it != interval_deque_.DataEnd(); ++it) { - if (it->offset >= end) { - break; - } - if (!it->slice.empty() && - bytes_acked_.Contains(it->offset, it->offset + it->slice.length())) { - it->slice.Reset(); - } - } - return true; - } - auto it = buffered_slices_.begin(); - // Find it, such that buffered_slices_[it - 1].end < start <= - // buffered_slices_[it].end. - if (it == buffered_slices_.end() || it->slice.empty()) { + auto it = interval_deque_.DataBegin(); + if (it == interval_deque_.DataEnd() || it->slice.empty()) { QUIC_BUG << "Trying to ack stream data [" << start << ", " << end << "), " - << (it == buffered_slices_.end() + << (it == interval_deque_.DataEnd() ? "and there is no outstanding data." : "and the first slice is empty."); return false; } - if (start >= it->offset + it->slice.length() || start < it->offset) { + if (!it->interval().Contains(start)) { // Slow path that not the earliest outstanding data gets acked. - it = std::lower_bound(buffered_slices_.begin(), buffered_slices_.end(), - start, CompareOffset()); + it = std::lower_bound(interval_deque_.DataBegin(), + interval_deque_.DataEnd(), start, CompareOffset()); } - if (it == buffered_slices_.end() || it->slice.empty()) { - QUIC_BUG << "Offset " << start - << " does not exist or it has already been acked."; + if (it == interval_deque_.DataEnd() || it->slice.empty()) { + QUIC_BUG << "Offset " << start << " with iterator offset: " << it->offset + << (it == interval_deque_.DataEnd() ? " does not exist." + : " has already been acked."); return false; } - for (; it != buffered_slices_.end(); ++it) { + for (; it != interval_deque_.DataEnd(); ++it) { if (it->offset >= end) { break; } @@ -361,34 +258,14 @@ } void QuicStreamSendBuffer::CleanUpBufferedSlices() { - if (interval_deque_active_) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_interval_deque, 4, 5); - while (!interval_deque_.Empty() && - interval_deque_.DataBegin()->slice.empty()) { - QUIC_BUG_IF(interval_deque_.DataBegin()->offset > current_end_offset_) - << "Fail to pop front from interval_deque_. Front element contained " - "a slice whose data has not all be written. Front offset " - << interval_deque_.DataBegin()->offset << " length " - << interval_deque_.DataBegin()->slice.length(); - interval_deque_.PopFront(); - } - return; - } - while (!buffered_slices_.empty() && buffered_slices_.front().slice.empty()) { - // Remove data which stops waiting for acks. Please note, mem slices can - // be released out of order, but send buffer is cleaned up in order. - QUIC_BUG_IF(write_index_ == 0) - << "Fail to advance current_write_slice_. It points to the slice " - "whose data has all be written and ACK'ed or ignored. " - "current_write_slice_ offset " - << buffered_slices_[write_index_].offset << " length " - << buffered_slices_[write_index_].slice.length(); - if (write_index_ > 0) { - // If write index is pointing to any slice, reduce the index as the - // slices are all shifted to the left by one. - --write_index_; - } - buffered_slices_.pop_front(); + while (!interval_deque_.Empty() && + interval_deque_.DataBegin()->slice.empty()) { + QUIC_BUG_IF(interval_deque_.DataBegin()->offset > current_end_offset_) + << "Fail to pop front from interval_deque_. Front element contained " + "a slice whose data has not all be written. Front offset " + << interval_deque_.DataBegin()->offset << " length " + << interval_deque_.DataBegin()->slice.length(); + interval_deque_.PopFront(); } } @@ -400,12 +277,7 @@ } size_t QuicStreamSendBuffer::size() const { - if (interval_deque_active_) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_interval_deque, 5, 5); - return interval_deque_.Size(); - } else { - return buffered_slices_.size(); - } + return interval_deque_.Size(); } } // namespace quic
diff --git a/quic/core/quic_stream_send_buffer.h b/quic/core/quic_stream_send_buffer.h index be0aa00..7144894 100644 --- a/quic/core/quic_stream_send_buffer.h +++ b/quic/core/quic_stream_send_buffer.h
@@ -145,8 +145,6 @@ // Cleanup empty slices in order from buffered_slices_. void CleanUpBufferedSlices(); - bool interval_deque_active_; - QuicCircularDeque<BufferedSlice> buffered_slices_; // |current_end_offset_| stores the end offset of the current slice to ensure // data isn't being written out of order when using the |interval_deque_|. QuicStreamOffset current_end_offset_;
diff --git a/quic/core/quic_stream_send_buffer_test.cc b/quic/core/quic_stream_send_buffer_test.cc index 35d8901..6232887 100644 --- a/quic/core/quic_stream_send_buffer_test.cc +++ b/quic/core/quic_stream_send_buffer_test.cc
@@ -46,14 +46,8 @@ memset(buffer2.get(), 'd', 768); QuicMemSlice slice2(std::move(buffer2), 768); - if (!GetQuicReloadableFlag(quic_interval_deque)) { - // Index starts from not pointing to any slice. - EXPECT_EQ(nullptr, - QuicStreamSendBufferPeer::CurrentWriteSlice(&send_buffer_)); - } else { - // The stream offset should be 0 since nothing is written. - EXPECT_EQ(0u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); - } + // The stream offset should be 0 since nothing is written. + EXPECT_EQ(0u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); // Save all data. SetQuicFlag(FLAGS_quic_send_buffer_max_data_slice_size, 1024); @@ -143,27 +137,14 @@ EXPECT_EQ(copy1 + copy2, quiche::QuicheStringPiece(buf + 1024, 2048)); // Write new data. - if (!GetQuicReloadableFlag(quic_interval_deque)) { - EXPECT_EQ(2, QuicStreamSendBufferPeer::write_index(&send_buffer_)); - ASSERT_TRUE(send_buffer_.WriteStreamData(2048, 50, &writer)); - EXPECT_EQ(std::string(50, 'c'), - quiche::QuicheStringPiece(buf + 1024 + 2048, 50)); - EXPECT_EQ(2, QuicStreamSendBufferPeer::write_index(&send_buffer_)); - ASSERT_TRUE(send_buffer_.WriteStreamData(2048, 1124, &writer)); - EXPECT_EQ(copy3, quiche::QuicheStringPiece(buf + 1024 + 2048 + 50, 1124)); - EXPECT_EQ(3, QuicStreamSendBufferPeer::write_index(&send_buffer_)); - } else { - // if |quic_interval_deque| is true then |write_index_| has no value. - // Instead we ensure the |current_end_offet| has an appropriate value. - EXPECT_EQ(2048u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); - ASSERT_TRUE(send_buffer_.WriteStreamData(2048, 50, &writer)); - EXPECT_EQ(std::string(50, 'c'), - quiche::QuicheStringPiece(buf + 1024 + 2048, 50)); - EXPECT_EQ(3072u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); - ASSERT_TRUE(send_buffer_.WriteStreamData(2048, 1124, &writer)); - EXPECT_EQ(copy3, quiche::QuicheStringPiece(buf + 1024 + 2048 + 50, 1124)); - EXPECT_EQ(3840u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); - } + EXPECT_EQ(2048u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); + ASSERT_TRUE(send_buffer_.WriteStreamData(2048, 50, &writer)); + EXPECT_EQ(std::string(50, 'c'), + quiche::QuicheStringPiece(buf + 1024 + 2048, 50)); + EXPECT_EQ(3072u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); + ASSERT_TRUE(send_buffer_.WriteStreamData(2048, 1124, &writer)); + EXPECT_EQ(copy3, quiche::QuicheStringPiece(buf + 1024 + 2048 + 50, 1124)); + EXPECT_EQ(3840u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); } TEST_F(QuicStreamSendBufferTest, RemoveStreamFrame) { @@ -304,73 +285,37 @@ EXPECT_TRUE(send_buffer_.IsStreamDataOutstanding(400, 800)); } -// TODO(b/144690240): Rename to EndOffset when deprecating --quic_interval_deque -TEST_F(QuicStreamSendBufferTest, CurrentWriteIndex) { +TEST_F(QuicStreamSendBufferTest, EndOffset) { char buf[4000]; QuicDataWriter writer(4000, buf, quiche::HOST_BYTE_ORDER); - if (!GetQuicReloadableFlag(quic_interval_deque)) { - // With data buffered, index points to the 1st slice of data. - EXPECT_EQ( - 0u, QuicStreamSendBufferPeer::CurrentWriteSlice(&send_buffer_)->offset); - } else { - EXPECT_EQ(1024u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); - } + + EXPECT_EQ(1024u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); ASSERT_TRUE(send_buffer_.WriteStreamData(0, 1024, &writer)); - // Wrote all data on 1st slice, index points to next slice. - if (!GetQuicReloadableFlag(quic_interval_deque)) { - EXPECT_EQ( - 1024u, - QuicStreamSendBufferPeer::CurrentWriteSlice(&send_buffer_)->offset); - } else { - // Last offset we've seen is 1024 - EXPECT_EQ(1024u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); - } + // Last offset we've seen is 1024 + EXPECT_EQ(1024u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); + ASSERT_TRUE(send_buffer_.WriteStreamData(1024, 512, &writer)); - // Last write didn't finish a whole slice. Index remains. - if (!GetQuicReloadableFlag(quic_interval_deque)) { - EXPECT_EQ( - 1024u, - QuicStreamSendBufferPeer::CurrentWriteSlice(&send_buffer_)->offset); - } else { - // Last offset is now 2048 as that's the end of the next slice. - EXPECT_EQ(2048u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); - } + // Last offset is now 2048 as that's the end of the next slice. + EXPECT_EQ(2048u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); send_buffer_.OnStreamDataConsumed(1024); // If data in 1st slice gets ACK'ed, it shouldn't change the indexed slice QuicByteCount newly_acked_length; EXPECT_TRUE(send_buffer_.OnStreamDataAcked(0, 1024, &newly_acked_length)); - if (!GetQuicReloadableFlag(quic_interval_deque)) { - EXPECT_EQ( - 1024u, - QuicStreamSendBufferPeer::CurrentWriteSlice(&send_buffer_)->offset); - } else { - // Last offset is still 2048. - EXPECT_EQ(2048u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); - } + // Last offset is still 2048. + EXPECT_EQ(2048u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); ASSERT_TRUE( send_buffer_.WriteStreamData(1024 + 512, 3840 - 1024 - 512, &writer)); - // After writing all buffered data, index become invalid again. - if (!GetQuicReloadableFlag(quic_interval_deque)) { - EXPECT_EQ(nullptr, - QuicStreamSendBufferPeer::CurrentWriteSlice(&send_buffer_)); - } else { - // Last offset is end offset of last slice. - EXPECT_EQ(3840u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); - } + + // Last offset is end offset of last slice. + EXPECT_EQ(3840u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); QuicUniqueBufferPtr buffer = MakeUniqueBuffer(&allocator_, 60); memset(buffer.get(), 'e', 60); QuicMemSlice slice(std::move(buffer), 60); send_buffer_.SaveMemSlice(std::move(slice)); - if (!GetQuicReloadableFlag(quic_interval_deque)) { - // With new data, index points to the new data. - EXPECT_EQ( - 3840u, - QuicStreamSendBufferPeer::CurrentWriteSlice(&send_buffer_)->offset); - } else { - EXPECT_EQ(3840u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); - } + + EXPECT_EQ(3840u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_)); } TEST_F(QuicStreamSendBufferTest, SaveMemSliceSpan) {
diff --git a/quic/test_tools/quic_stream_send_buffer_peer.cc b/quic/test_tools/quic_stream_send_buffer_peer.cc index 2135d3f..7257915 100644 --- a/quic/test_tools/quic_stream_send_buffer_peer.cc +++ b/quic/test_tools/quic_stream_send_buffer_peer.cc
@@ -16,8 +16,6 @@ send_buffer->stream_offset_ = stream_offset; } -// TODO(b/144690240): Remove CurrentWriteSlice when deprecating -// --quic_interval_deque // static const BufferedSlice* QuicStreamSendBufferPeer::CurrentWriteSlice( QuicStreamSendBuffer* send_buffer) { @@ -26,34 +24,21 @@ if (wi == -1) { return nullptr; } - if (GetQuicReloadableFlag(quic_interval_deque)) { - return QuicIntervalDequePeer::GetItem(&send_buffer->interval_deque_, wi); - } else { - return &send_buffer->buffered_slices_[wi]; - } + return QuicIntervalDequePeer::GetItem(&send_buffer->interval_deque_, wi); } QuicStreamOffset QuicStreamSendBufferPeer::EndOffset( QuicStreamSendBuffer* send_buffer) { - if (GetQuicReloadableFlag(quic_interval_deque)) { - return send_buffer->current_end_offset_; - } - return 0; + return send_buffer->current_end_offset_; } // static QuicByteCount QuicStreamSendBufferPeer::TotalLength( QuicStreamSendBuffer* send_buffer) { QuicByteCount length = 0; - if (GetQuicReloadableFlag(quic_interval_deque)) { - for (auto slice = send_buffer->interval_deque_.DataBegin(); - slice != send_buffer->interval_deque_.DataEnd(); ++slice) { - length += slice->slice.length(); - } - } else { - for (const auto& slice : send_buffer->buffered_slices_) { - length += slice.slice.length(); - } + for (auto slice = send_buffer->interval_deque_.DataBegin(); + slice != send_buffer->interval_deque_.DataEnd(); ++slice) { + length += slice->slice.length(); } return length; } @@ -61,11 +46,7 @@ // static int32_t QuicStreamSendBufferPeer::write_index( QuicStreamSendBuffer* send_buffer) { - if (send_buffer->interval_deque_active_) { - return QuicIntervalDequePeer::GetCachedIndex(&send_buffer->interval_deque_); - } else { - return send_buffer->write_index_; - } + return QuicIntervalDequePeer::GetCachedIndex(&send_buffer->interval_deque_); } } // namespace test