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
