gfe-relnote: In QUIC, let write_blocked_list be able to use HTTP2 priorities. Not used yet. Not Protected. PiperOrigin-RevId: 260156145 Change-Id: I49e1e7547ed3fdf302f6b3c182d00b4c801d1aed
diff --git a/quic/core/quic_write_blocked_list.cc b/quic/core/quic_write_blocked_list.cc index d7fe3c0..c9b9c22 100644 --- a/quic/core/quic_write_blocked_list.cc +++ b/quic/core/quic_write_blocked_list.cc
@@ -6,14 +6,18 @@ #include "net/third_party/quiche/src/quic/platform/api/quic_flag_utils.h" #include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" +#include "net/third_party/quiche/src/spdy/core/priority_write_scheduler.h" namespace quic { QuicWriteBlockedList::QuicWriteBlockedList(QuicTransportVersion version) - : priority_write_scheduler_(QuicVersionUsesCryptoFrames(version) - ? std::numeric_limits<QuicStreamId>::max() - : 0), - last_priority_popped_(0) { + : priority_write_scheduler_( + QuicMakeUnique<spdy::PriorityWriteScheduler<QuicStreamId>>( + QuicVersionUsesCryptoFrames(version) + ? std::numeric_limits<QuicStreamId>::max() + : 0)), + last_priority_popped_(0), + scheduler_type_(spdy::WriteSchedulerType::SPDY) { memset(batch_write_stream_id_, 0, sizeof(batch_write_stream_id_)); memset(bytes_left_for_batch_write_, 0, sizeof(bytes_left_for_batch_write_)); }
diff --git a/quic/core/quic_write_blocked_list.h b/quic/core/quic_write_blocked_list.h index 4b49816..7b7c5a8 100644 --- a/quic/core/quic_write_blocked_list.h +++ b/quic/core/quic_write_blocked_list.h
@@ -9,10 +9,12 @@ #include <cstdint> #include "net/third_party/quiche/src/quic/core/quic_packets.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h" #include "net/third_party/quiche/src/quic/platform/api/quic_containers.h" #include "net/third_party/quiche/src/quic/platform/api/quic_export.h" #include "net/third_party/quiche/src/quic/platform/api/quic_map_util.h" -#include "net/third_party/quiche/src/spdy/core/priority_write_scheduler.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" +#include "net/third_party/quiche/src/spdy/core/http2_priority_write_scheduler.h" namespace quic { @@ -21,7 +23,7 @@ // Crypto stream > Headers stream > Data streams by requested priority. class QUIC_EXPORT_PRIVATE QuicWriteBlockedList { private: - typedef spdy::PriorityWriteScheduler<QuicStreamId> QuicPriorityWriteScheduler; + typedef spdy::WriteScheduler<QuicStreamId> QuicPriorityWriteScheduler; public: explicit QuicWriteBlockedList(QuicTransportVersion version); @@ -30,7 +32,7 @@ ~QuicWriteBlockedList(); bool HasWriteBlockedDataStreams() const { - return priority_write_scheduler_.HasReadyStreams(); + return priority_write_scheduler_->HasReadyStreams(); } bool HasWriteBlockedSpecialStream() const { @@ -43,7 +45,7 @@ size_t NumBlockedStreams() const { return NumBlockedSpecialStreams() + - priority_write_scheduler_.NumReadyStreams(); + priority_write_scheduler_->NumReadyStreams(); } bool ShouldYield(QuicStreamId id) const { @@ -58,11 +60,28 @@ } } - return priority_write_scheduler_.ShouldYield(id); + return priority_write_scheduler_->ShouldYield(id); } - // Pops the highest priorty stream, special casing crypto and headers streams. - // Latches the most recently popped data stream for batch writing purposes. + // Uses HTTP2 (tree-style) priority scheduler. This can only be called before + // any stream is registered. + bool UseHttp2PriorityScheduler() { + if (scheduler_type_ == spdy::WriteSchedulerType::HTTP2) { + return true; + } + if (priority_write_scheduler_->NumRegisteredStreams() != 0) { + QUIC_BUG << "Cannot switch scheduler with registered streams"; + return false; + } + priority_write_scheduler_ = + QuicMakeUnique<spdy::Http2PriorityWriteScheduler<QuicStreamId>>(); + scheduler_type_ = spdy::WriteSchedulerType::HTTP2; + return true; + } + + // Pops the highest priority stream, special casing crypto and headers + // streams. Latches the most recently popped data stream for batch writing + // purposes. QuicStreamId PopFront() { QuicStreamId static_stream_id; if (static_stream_collection_.UnblockFirstBlocked(&static_stream_id)) { @@ -70,12 +89,16 @@ } const auto id_and_precedence = - priority_write_scheduler_.PopNextReadyStreamAndPrecedence(); + priority_write_scheduler_->PopNextReadyStreamAndPrecedence(); const QuicStreamId id = std::get<0>(id_and_precedence); + if (scheduler_type_ == spdy::WriteSchedulerType::HTTP2) { + // No batch writing logic when using HTTP2 priorities. + return id; + } const spdy::SpdyPriority priority = std::get<1>(id_and_precedence).spdy3_priority(); - if (!priority_write_scheduler_.HasReadyStreams()) { + if (!priority_write_scheduler_->HasReadyStreams()) { // If no streams are blocked, don't bother latching. This stream will be // the first popped for its priority anyway. batch_write_stream_id_[priority] = 0; @@ -93,13 +116,14 @@ void RegisterStream(QuicStreamId stream_id, bool is_static_stream, const spdy::SpdyStreamPrecedence& precedence) { - DCHECK(!priority_write_scheduler_.StreamRegistered(stream_id)); + DCHECK(!priority_write_scheduler_->StreamRegistered(stream_id) && + PrecedenceMatchesSchedulerType(precedence)); if (is_static_stream) { static_stream_collection_.Register(stream_id); return; } - priority_write_scheduler_.RegisterStream(stream_id, precedence); + priority_write_scheduler_->RegisterStream(stream_id, precedence); } void UnregisterStream(QuicStreamId stream_id, bool is_static) { @@ -107,16 +131,21 @@ static_stream_collection_.Unregister(stream_id); return; } - priority_write_scheduler_.UnregisterStream(stream_id); + priority_write_scheduler_->UnregisterStream(stream_id); } void UpdateStreamPriority(QuicStreamId stream_id, const spdy::SpdyStreamPrecedence& new_precedence) { - DCHECK(!static_stream_collection_.IsRegistered(stream_id)); - priority_write_scheduler_.UpdateStreamPrecedence(stream_id, new_precedence); + DCHECK(!static_stream_collection_.IsRegistered(stream_id) && + PrecedenceMatchesSchedulerType(new_precedence)); + priority_write_scheduler_->UpdateStreamPrecedence(stream_id, + new_precedence); } void UpdateBytesForStream(QuicStreamId stream_id, size_t bytes) { + if (scheduler_type_ == spdy::WriteSchedulerType::HTTP2) { + return; + } if (batch_write_stream_id_[last_priority_popped_] == stream_id) { // If this was the last data stream popped by PopFront, update the // bytes remaining in its batch write. @@ -135,9 +164,10 @@ } bool push_front = + scheduler_type_ == spdy::WriteSchedulerType::SPDY && stream_id == batch_write_stream_id_[last_priority_popped_] && bytes_left_for_batch_write_[last_priority_popped_] > 0; - priority_write_scheduler_.MarkStreamReady(stream_id, push_front); + priority_write_scheduler_->MarkStreamReady(stream_id, push_front); } // Returns true if stream with |stream_id| is write blocked. @@ -148,11 +178,19 @@ } } - return priority_write_scheduler_.IsStreamReady(stream_id); + return priority_write_scheduler_->IsStreamReady(stream_id); } private: - QuicPriorityWriteScheduler priority_write_scheduler_; + bool PrecedenceMatchesSchedulerType( + const spdy::SpdyStreamPrecedence& precedence) { + if (precedence.is_spdy3_priority()) { + return scheduler_type_ == spdy::WriteSchedulerType::SPDY; + } + return scheduler_type_ == spdy::WriteSchedulerType::HTTP2; + } + + std::unique_ptr<QuicPriorityWriteScheduler> priority_write_scheduler_; // If performing batch writes, this will be the stream ID of the stream doing // batch writes for this priority level. We will allow this stream to write @@ -252,6 +290,8 @@ }; StaticStreamCollection static_stream_collection_; + + spdy::WriteSchedulerType scheduler_type_; }; } // namespace quic
diff --git a/quic/core/quic_write_blocked_list_test.cc b/quic/core/quic_write_blocked_list_test.cc index f26fc56..ca3433b 100644 --- a/quic/core/quic_write_blocked_list_test.cc +++ b/quic/core/quic_write_blocked_list_test.cc
@@ -7,6 +7,7 @@ #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" #include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h" +using spdy::kHttp2DefaultStreamWeight; using spdy::kV3HighestPriority; using spdy::kV3LowestPriority; @@ -14,28 +15,68 @@ namespace test { namespace { -class QuicWriteBlockedListTest : public QuicTest { +const bool kExclusiveBit = true; + +class QuicWriteBlockedListTest : public QuicTestWithParam<bool> { public: QuicWriteBlockedListTest() - : write_blocked_list_(AllSupportedVersions()[0].transport_version) {} + : write_blocked_list_(AllSupportedVersions()[0].transport_version) { + if (GetParam()) { + write_blocked_list_.UseHttp2PriorityScheduler(); + } + } protected: QuicWriteBlockedList write_blocked_list_; }; -TEST_F(QuicWriteBlockedListTest, PriorityOrder) { - // Mark streams blocked in roughly reverse priority order, and - // verify that streams are sorted. - write_blocked_list_.RegisterStream( - 40, false, spdy::SpdyStreamPrecedence(kV3LowestPriority)); - write_blocked_list_.RegisterStream( - 23, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); - write_blocked_list_.RegisterStream( - 17, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); - write_blocked_list_.RegisterStream( - 1, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); - write_blocked_list_.RegisterStream( - 3, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); +INSTANTIATE_TEST_SUITE_P(Tests, QuicWriteBlockedListTest, testing::Bool()); + +TEST_P(QuicWriteBlockedListTest, PriorityOrder) { + if (GetParam()) { + /* + 0 + | + 23 + | + 17 + | + 40 + */ + write_blocked_list_.RegisterStream( + 17, false, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 40, false, + spdy::SpdyStreamPrecedence(17, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 23, false, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 1, true, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 3, true, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + } else { + // Mark streams blocked in roughly reverse priority order, and + // verify that streams are sorted. + write_blocked_list_.RegisterStream( + 40, false, spdy::SpdyStreamPrecedence(kV3LowestPriority)); + write_blocked_list_.RegisterStream( + 23, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + write_blocked_list_.RegisterStream( + 17, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + write_blocked_list_.RegisterStream( + 1, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + write_blocked_list_.RegisterStream( + 3, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + } write_blocked_list_.AddStream(40); EXPECT_TRUE(write_blocked_list_.IsStreamBlocked(40)); @@ -74,9 +115,16 @@ EXPECT_FALSE(write_blocked_list_.HasWriteBlockedDataStreams()); } -TEST_F(QuicWriteBlockedListTest, CryptoStream) { - write_blocked_list_.RegisterStream( - 1, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); +TEST_P(QuicWriteBlockedListTest, CryptoStream) { + if (GetParam()) { + write_blocked_list_.RegisterStream( + 1, true, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + } else { + write_blocked_list_.RegisterStream( + 1, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + } write_blocked_list_.AddStream(1); EXPECT_EQ(1u, write_blocked_list_.NumBlockedStreams()); @@ -86,9 +134,16 @@ EXPECT_FALSE(write_blocked_list_.HasWriteBlockedSpecialStream()); } -TEST_F(QuicWriteBlockedListTest, HeadersStream) { - write_blocked_list_.RegisterStream( - 3, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); +TEST_P(QuicWriteBlockedListTest, HeadersStream) { + if (GetParam()) { + write_blocked_list_.RegisterStream( + 3, true, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + } else { + write_blocked_list_.RegisterStream( + 3, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + } write_blocked_list_.AddStream(3); EXPECT_EQ(1u, write_blocked_list_.NumBlockedStreams()); @@ -98,11 +153,22 @@ EXPECT_FALSE(write_blocked_list_.HasWriteBlockedSpecialStream()); } -TEST_F(QuicWriteBlockedListTest, VerifyHeadersStream) { - write_blocked_list_.RegisterStream( - 5, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); - write_blocked_list_.RegisterStream( - 3, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); +TEST_P(QuicWriteBlockedListTest, VerifyHeadersStream) { + if (GetParam()) { + write_blocked_list_.RegisterStream( + 5, false, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 3, true, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + } else { + write_blocked_list_.RegisterStream( + 5, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + write_blocked_list_.RegisterStream( + 3, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + } write_blocked_list_.AddStream(5); write_blocked_list_.AddStream(3); @@ -118,13 +184,20 @@ EXPECT_FALSE(write_blocked_list_.HasWriteBlockedDataStreams()); } -TEST_F(QuicWriteBlockedListTest, NoDuplicateEntries) { +TEST_P(QuicWriteBlockedListTest, NoDuplicateEntries) { // Test that QuicWriteBlockedList doesn't allow duplicate entries. // Try to add a stream to the write blocked list multiple times at the same // priority. const QuicStreamId kBlockedId = 3 + 2; - write_blocked_list_.RegisterStream( - kBlockedId, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + if (GetParam()) { + write_blocked_list_.RegisterStream( + kBlockedId, false, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + } else { + write_blocked_list_.RegisterStream( + kBlockedId, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + } write_blocked_list_.AddStream(kBlockedId); write_blocked_list_.AddStream(kBlockedId); write_blocked_list_.AddStream(kBlockedId); @@ -139,7 +212,10 @@ EXPECT_FALSE(write_blocked_list_.HasWriteBlockedDataStreams()); } -TEST_F(QuicWriteBlockedListTest, BatchingWrites) { +TEST_P(QuicWriteBlockedListTest, BatchingWrites) { + if (GetParam()) { + return; + } const QuicStreamId id1 = 3 + 2; const QuicStreamId id2 = id1 + 2; const QuicStreamId id3 = id2 + 2; @@ -192,18 +268,62 @@ EXPECT_EQ(id1, write_blocked_list_.PopFront()); } -TEST_F(QuicWriteBlockedListTest, Ceding) { - write_blocked_list_.RegisterStream( - 15, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); - write_blocked_list_.RegisterStream( - 16, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); - write_blocked_list_.RegisterStream(5, false, spdy::SpdyStreamPrecedence(5)); - write_blocked_list_.RegisterStream(4, false, spdy::SpdyStreamPrecedence(5)); - write_blocked_list_.RegisterStream(7, false, spdy::SpdyStreamPrecedence(7)); - write_blocked_list_.RegisterStream( - 1, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); - write_blocked_list_.RegisterStream( - 3, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); +TEST_P(QuicWriteBlockedListTest, Ceding) { + if (GetParam()) { + /* + 0 + | + 15 + | + 16 + | + 5 + | + 4 + | + 7 + */ + write_blocked_list_.RegisterStream( + 15, false, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 16, false, + spdy::SpdyStreamPrecedence(15, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 4, false, + spdy::SpdyStreamPrecedence(16, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 5, false, + spdy::SpdyStreamPrecedence(16, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 7, false, + spdy::SpdyStreamPrecedence(4, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 1, true, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + write_blocked_list_.RegisterStream( + 3, true, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, + kExclusiveBit)); + } else { + write_blocked_list_.RegisterStream( + 15, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + write_blocked_list_.RegisterStream( + 16, false, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + write_blocked_list_.RegisterStream(5, false, spdy::SpdyStreamPrecedence(5)); + write_blocked_list_.RegisterStream(4, false, spdy::SpdyStreamPrecedence(5)); + write_blocked_list_.RegisterStream(7, false, spdy::SpdyStreamPrecedence(7)); + write_blocked_list_.RegisterStream( + 1, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + write_blocked_list_.RegisterStream( + 3, true, spdy::SpdyStreamPrecedence(kV3HighestPriority)); + } // When nothing is on the list, nothing yields. EXPECT_FALSE(write_blocked_list_.ShouldYield(5)); @@ -241,6 +361,66 @@ EXPECT_FALSE(write_blocked_list_.ShouldYield(1)); } +TEST_P(QuicWriteBlockedListTest, UpdateStreamPriority) { + if (!GetParam()) { + return; + } + /* + 0 + | + 5 + | + 7 + | + 9 + | + 11 + */ + write_blocked_list_.RegisterStream( + 5, false, + spdy::SpdyStreamPrecedence(0, kHttp2DefaultStreamWeight, kExclusiveBit)); + write_blocked_list_.RegisterStream( + 7, false, + spdy::SpdyStreamPrecedence(5, kHttp2DefaultStreamWeight, kExclusiveBit)); + write_blocked_list_.RegisterStream( + 9, false, + spdy::SpdyStreamPrecedence(7, kHttp2DefaultStreamWeight, kExclusiveBit)); + write_blocked_list_.RegisterStream( + 11, false, + spdy::SpdyStreamPrecedence(9, kHttp2DefaultStreamWeight, kExclusiveBit)); + + write_blocked_list_.AddStream(7); + EXPECT_FALSE(write_blocked_list_.ShouldYield(5)); + EXPECT_TRUE(write_blocked_list_.ShouldYield(9)); + EXPECT_TRUE(write_blocked_list_.ShouldYield(11)); + + // Update 9's priority. + if (GetParam()) { + /* + 0 + | + 5 + / \ + 7 9 + | + 11 + */ + write_blocked_list_.UpdateStreamPriority( + 9, spdy::SpdyStreamPrecedence(5, kHttp2DefaultStreamWeight, + kExclusiveBit)); + } else { + write_blocked_list_.UpdateStreamPriority(9, spdy::SpdyStreamPrecedence(1)); + } + EXPECT_FALSE(write_blocked_list_.ShouldYield(5)); + // Verify 9 now does not yield to 7. + EXPECT_FALSE(write_blocked_list_.ShouldYield(9)); + EXPECT_FALSE(write_blocked_list_.ShouldYield(11)); + + write_blocked_list_.AddStream(9); + // Verify 11 yield to 9. + EXPECT_TRUE(write_blocked_list_.ShouldYield(11)); +} + } // namespace } // namespace test } // namespace quic